GNOME Bugzilla – Bug 739750
Fix handling of windows with sizes that aren't a multiple of window_scale
Last modified: 2014-11-20 11:42:36 UTC
The constrain to maximize a window will typical override the resize increments we specify, so we can actually end up with windows that aren't a multiple of window_scale. The old handling of this was that we rounded down, and got a black line at the bottom and right - except for with GL when we got junk at the *top* and right, but it's better to round up and truncate at the bottom and right. Getting the truncation right for GL is a bit tricky and requires us to track the actual size of the window in X server pixels.
Created attachment 290120 [details] [review] x11: round the scaled size *up* when we get a ConfigureNotify Although we specify a resize increment to try and get a size that is a multiple of the window scale, maximization typically wins over the resize increment, so the window might be odd sized. Round *up* in this case, rather than down, since it's better to truncate a line or two at the bottom and right of the window rather than have a line or two that we don't know what to do with.
Created attachment 290121 [details] [review] Revive GdkGLContext.update() and rename to update_viewport() The update virtual function for GdkGLContext was unused and bit-rotted; replace it with a similar update_viewport() that calls glViewport() and add a default implementation.
Created attachment 290122 [details] [review] x11: Keep track of the exact size in X pixels of windows Keep track of the exact size of X windows in underlying pixels; we generally use the scaled size instead, but to properly handle the GL viewport for windows that aren't a multiple of window_scale, we need to know the real size.
Created attachment 290123 [details] [review] GdkX11GLContext: set the viewport if the window size isn't a multiple of window_scale Our philosophy with windows that aren't a multiple of window_scale is that we truncate at the bottom and right. Because of GL's choice of coordinates (0,0 at bottom-right), we need to adjust our call to glViewport to make this work correctly for windows with GL content.
Review of attachment 290120 [details] [review]: This makes a lot of sense to me.
Review of attachment 290121 [details] [review]: looks good ::: gdk/gdkglcontext.c @@ +235,3 @@ + int wh = gdk_window_get_height (priv->window) * scale; + + glViewport (0, 0, ww, wh); The old update code had some debug output when setting glViewport. I wonder if that is ever useful, probably not...
Review of attachment 290122 [details] [review]: I wonder if we should also use the unscaled size in the _gdk_x11_window_update_size and gdk_x11_ref_cairo_surface? Will cairo be confused if it thinks the window is taller than it really is? Also, we should round up in gdk_x11_window_get_frame_extents() too, no?
Review of attachment 290123 [details] [review]: looks good
Review of attachment 290120 [details] [review]: I tried, this and it didn't quite work, because we're not changing the reported size in the Configure event to also be rounded up.
I fixed that, but even then i have issues. If the window is an odd number of pixels tall, then the fast path of gdk_cairo_draw_from_gl() fails, because it needs to use window coordinates for blit and quad drawing, and these needs to flip the y coordinates, which fails because the height we have is one pixel taller than what gl uses.
(The failure mode now is garbage at the bottom, rather than at the top though).
In general, the approach of just setting the viewport isn't good enough. That only affects the normalized coords => framebuffer coords transform (i.e. the coordinates when you're drawing a quad), but it doesn't affect things like scissor or framebuffer blitting coordinates. The proper solution needs to have the actual physical framebuffer/window height in the GL code. I'm attaching a new series that has this fix and a few others.
Created attachment 291078 [details] [review] x11: round the scaled size *up* when we get a ConfigureNotify Although we specify a resize increment to try and get a size that is a multiple of the window scale, maximization typically wins over the resize increment, so the window might be odd sized. Round *up* in this case, rather than down, since it's better to truncate a line or two at the bottom and right of the window rather than have a line or two that we don't know what to do with.
Created attachment 291079 [details] [review] x11: Keep track of the exact size in X pixels of windows Keep track of the exact size of X windows in underlying pixels; we generally use the scaled size instead, but to properly handle the GL viewport for windows that aren't a multiple of window_scale, we need to know the real size.
Created attachment 291080 [details] [review] x11: Return the exact pixel coverage in get_frame_extents Rather than just rounding down the position *and* the size separately we correctly calculate a rectangle in scaled window coords that fully covers the real window size. This really only makes a difference when the window size/position isn't a multiple of the window scale.
Created attachment 291081 [details] [review] Add gdk_window_get_unscaled_size This is required for the X backend GL integration. If the window has a height that is not a multiple of the window scale we can't properly do the y coordinate flipping that GL needs. Other backends can ignore this and use the default implementation.
Created attachment 291082 [details] [review] GL: Fix GL Y coordinate flipping to use unscaled window height This is needed in the edge case where the X11 backend rounded the actual size, and the GL flipping really needs the correct window height to do proper Y coordinate flipping.
Created attachment 291083 [details] [review] GdkGLContext: Remove unused update vfunc The update virtual function for GdkGLContext is unused and is a leftover from a previous GL approach. Just remove it.
Attachment 291078 [details] pushed as 608c254 - x11: round the scaled size *up* when we get a ConfigureNotify Attachment 291079 [details] pushed as bd643e0 - x11: Keep track of the exact size in X pixels of windows Attachment 291080 [details] pushed as 788478d - x11: Return the exact pixel coverage in get_frame_extents Attachment 291081 [details] pushed as 1eb3b34 - Add gdk_window_get_unscaled_size Attachment 291082 [details] pushed as 800c712 - GL: Fix GL Y coordinate flipping to use unscaled window height Attachment 291083 [details] pushed as cf94da2 - GdkGLContext: Remove unused update vfunc