440950 - repainting problems with non-pixel-aligned controls with gtk

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Eli Friedman]

Attachment: Patch

Linux variant of bug 382458, painting issues with attachment 271058 [details].  Patch attached.

The patch removes an unnecessary translation, and significantly simplifies the code by unifying the snapped and non-snapped codepaths.

Writing a reftest is rather difficult because as formulated, the testcase depends on having a scroll widget.  I'm not completely sure why; perhaps it has to do with the negative translation of the rendering context (added by nsViewManager::RenderViews).

I'm not entirely confident that the clip rect is correct for the non-snapped case; I'm having a difficult time figuring out the right way to calculate it.  That said, this patch doesn't cause any obvious breakage for this case.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+  bool snapXY = ctx->UserToDevicePixelSnapped(gfx_rect) &&

PRBool

+                ctx->UserToDevicePixelSnapped(gfx_clip);
+  if (snapXY) {

If one is snapped but the other isn't, things go wrong here because we'll carry on assuming they're both not snapped.

+  gdk_rect.x = NSToIntRound(aRect.X());
+  gdk_rect.y = NSToIntRound(aRect.Y());
   gdk_rect.width = NSToIntRound(aRect.Width());
   gdk_rect.height = NSToIntRound(aRect.Height());

Shouldn't this be rounding XMost/YMost instead of the width/height?

Other than that, I think this approach should work.

Comment 2

[Eli Friedman]

(In reply to comment #1)
> +  gdk_rect.x = NSToIntRound(aRect.X());
> +  gdk_rect.y = NSToIntRound(aRect.Y());
>    gdk_rect.width = NSToIntRound(aRect.Width());
>    gdk_rect.height = NSToIntRound(aRect.Height());
> 
> Shouldn't this be rounding XMost/YMost instead of the width/height?

When we're snapping, these are whole numbers, so it doesn't matter.  When we aren't snapping, for the widget size, I think making the width/height consistent ends up being more important than the consistency of pixel centers, since the pixel centers only really matter with snapping.  I really have no idea how to deal with the clip rect in the presence of general transforms.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

If the width and height are integers, then rounding xmost/ymost gives the same results as rounding width/height, so we should be fine almost all the time. If the width or height aren't integers I think it's important for the theme to cover the same area that a regular CSS image or rectangle would, which means rounding xmost/ymost.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 326079 [details] [diff] [review]
Patch

Karl's patch in bug 444837 should cover this. (I think I ended up agreeing with Karl and Eli over most of the issues here, sorry about that.)