550680 - Native theme elements have rightmost line stripped

Status: RESOLVED WORKSFORME

(Core :: Graphics, defect)

Description

[Bas Schouten (:bas.schouten)]

Attachment: Use RoundOut instead of Round on the native rect

After the fix to bug 549666, native theme elements can have their rightmost line of pixels stripped. See a screenshot on 549666. Using RoundOut() rather than Round() seems to fix this and have no undesirable effects.

Comment 1

[Markus Stange [:mstange]]

I don't think this will fix it without misaligning the control in some cases... Is it possible that the context has a fractional translation pushed to it?

FTR, similar bugs were bug 446463 on Mac and bug 404816 on Linux.

Comment 2

[Bas Schouten (:bas.schouten)]

(In reply to comment #1)
> I don't think this will fix it without misaligning the control in some cases...
> Is it possible that the context has a fractional translation pushed to it?
> 
> FTR, similar bugs were bug 446463 on Mac and bug 404816 on Linux.

A transform is used on the context to place the rect. So that should still cause proper alignment - in theory. I couldn't find any misalignments, but I might have overlooked something for sure. In any case it shouldn't misalign more than the current round, should it?

Comment 3

[Markus Stange [:mstange]]

(In reply to comment #2)
> A transform is used on the context to place the rect.

Is this the transform after the rounding? I'm concerned about a transform with unrounded values.

Comment 4

[Markus Stange [:mstange]]

Attachment: testcase

Comment 5

[Markus Stange [:mstange]]

Attachment: reference

Comment 6

[Markus Stange [:mstange]]

Does the testcase show the problem of this bug? And does your patch pass the reftest? (I don't have a Windows machine to test.)

Comment 7

[Bas Schouten (:bas.schouten)]

(In reply to comment #6)
> Does the testcase show the problem of this bug? And does your patch pass the
> reftest? (I don't have a Windows machine to test.)

It does not pass, your concerns are correct. The rounding affects only that a large enough area is drawn. It does not fix that in some of the area in your textcases the controls GDI draws are actually 1 pixel too small. I'll need to look into how to fix this, thanks!

Comment 9

[Bas Schouten (:bas.schouten)]

Attachment: RoundOut early

So the trick here is to RoundOut before creating the temporary surface. That way the widget will be drawn correctly. This makes the above mentioned reftest work, and seems to solve all remaining native theme element drawing problems.

Comment 10

[Bas Schouten (:bas.schouten)]

Pushed http://hg.mozilla.org/mozilla-central/rev/e3d045f4b570.

Comment 11

[u88484]

Verified fixed using hourly build Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a3pre) Gecko/20100312 Minefield/3.7a3pre ID:20100312053645

Not changing the status as I'm not sure if Bas has other work to do here since he didn't change the status to resolved fixed.

Comment 13

[Gary [:streetwolf52]]

I still lose popup menu separators every now and then.  I haven't been able to repro it.

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a4pre) Gecko/20100315 Minefield/3.7a4pre - Build ID: 20100315210142

Comment 14

[Gary [:streetwolf52]]

Popup menu separators are now being lost often with the latest build.

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a4pre) Gecko/20100317 Minefield/3.7a4pre - Build ID: 20100317125512

Comment 15

[Gary [:streetwolf52]]

I've finally discovered that losing menu separators every now and then only happens with d2d/dw enabled. Another consequence is choppy scrolling at times (see https://bugzilla.mozilla.org/show_bug.cgi?id=550495 )

Comment 16

[Gary [:streetwolf52]]

Still getting missing separators but not as often as before.  I can't repro it, it just happens.  Sometimes only the menu popups are affected while the context menus aren't.

Comment 17

[Gary [:streetwolf52]]

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a4pre) Gecko/20100403 Minefield/3.7a4pre - Build ID: 20100403071118

Comment 18

[Gary [:streetwolf52]]

Is this going to land soon?  I keep losing my menu separators.

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100429 Minefield/3.7a5pre Firefox/3.6 - Build ID: 20100429041232

Comment 19

[Bas Schouten (:bas.schouten)]

(In reply to comment #18)
> Is this going to land soon?  I keep losing my menu separators.
> 
> Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100429
> Minefield/3.7a5pre Firefox/3.6 - Build ID: 20100429041232

It did land, I'm pretty sure you're the only one still seeing problems :( Although I have no explanation why.

Comment 20

[Gary [:streetwolf52]]

No there is another (I'm a star wars fan).  Some other guy also has the problem.

Comment 21

[Gary [:streetwolf52]]

Bas.  Have a look at https://bugzilla.mozilla.org/show_bug.cgi?id=550768

Comment 22

[Gary [:streetwolf52]]

My separator problem has been solved by bug 551350.