Created attachment 223004 [details] [review]
windowManager: Disable the window dimmer when we close all attached dialogs

This saves some GLSL resources and an FBO.

Created attachment 223005 [details] [review]
js: Fix up for Clutter.Color changes

Spotted by darkxst on IRC.

Created attachment 223006 [details] [review]
magnifier: Use PointerWatcher to poll the mouse pointer

This gives us a more efficient method to watch the mouse pointer.

Created attachment 223007 [details] [review]
magnifier: Round the uiGroup to integer positions

This removes the jaggies from text, and other sorts of things.

Created attachment 223008 [details] [review]
magnifier: Don't set the size of the uiGroup

The magnifier shouldn't mess with global properties like this.

Created attachment 223009 [details] [review]
magnifier: Don't use some deprecated APIs

Stop using Clutter.Group and Clutter.Rectangle.

Review of attachment 223004 [details] [review]:

I don't like this "policy enabled" thing: if the attached-modal-dialog is false, we can set the effect to disabled like previous code did.
Then, when the undimming animation ends, the effect is set to disabled again.

Review of attachment 223005 [details] [review]:

Uhm, this is an API break.
I wonder if it should be fixed in clutter instead: if they wanted the constructor behaviour, they should have added "new_from_string" / "new_from_pixel".

Fine for the shell, if clutter doesn't change.

Review of attachment 223006 [details] [review]:

We really need to do better than this, for pointer tracking...

Review of attachment 223007 [details] [review]:

Sure

Review of attachment 223008 [details] [review]:

Makes sense, now that the uiGroup is forced to take the size of the screen by get_preferred_* overrides.

Review of attachment 223009 [details] [review]:

::: js/ui/magnifier.js
@@ +1085,3 @@
         // out of view (don't want to see desktop showing through).
+        this._background = new Clutter.Actor({ background_color: Main.DEFAULT_BACKGROUND_COLOR });
+        this._background.set_size(global.screen_width, global.screen_height);

new Clutter.Actor({ width: ...,
                    height: ...,
                    background_color: ... });

@@ +1455,3 @@
      */
     setColor: function(clutterColor) {
+        this._horizLeftHair.set_background_color(clutterColor);

this._horizLeftHair.background_color = ...

Review of attachment 223005 [details] [review]:

Technically, it's not an API break for Clutter. It's an API break for gobject-introspection.

http://git.gnome.org/browse/gobject-introspection/commit/giscanner/maintransformer.py?id=b0b4c98b31a23e6885b6c0785df93404b161b4b7

(In reply to comment #13)
> Review of attachment 223005 [details] [review]:
> 
> Technically, it's not an API break for Clutter. It's an API break for
> gobject-introspection.
> 
> http://git.gnome.org/browse/gobject-introspection/commit/giscanner/maintransformer.py?id=b0b4c98b31a23e6885b6c0785df93404b161b4b7

Ah. I thought it was because of the new (constructor) annotations.

Created attachment 223098 [details] [review]
windowManager: Disable the window dimmer when we close all attached dialogs

This saves some GLSL resources and an FBO.

How about something like this?

Attachment 223005 [details] pushed as e616877 - js: Fix up for Clutter.Color changes
Attachment 223006 [details] pushed as 8aed511 - magnifier: Use PointerWatcher to poll the mouse pointer
Attachment 223007 [details] pushed as 3fd0502 - magnifier: Round the uiGroup to integer positions
Attachment 223008 [details] pushed as 54e5ffc - magnifier: Don't set the size of the uiGroup
Attachment 223009 [details] pushed as c815979 - magnifier: Don't use some deprecated APIs


Pushed all the other fixes with the suggested changes.

Review of attachment 223098 [details] [review]:

This is not what I had in mind, but it works.

Attachment 223098 [details] pushed as f8805e8 - windowManager: Disable the window dimmer when we close all attached dialogs


I'm sort of curious what you had in mind, but whatever.