Closed
Bug 382049
Opened 18 years ago
Closed 17 years ago
Mac native form controls draw at incorrect positions in offscreen surfaces
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dbaron, Assigned: jtd)
References
Details
Attachments
(6 files, 2 obsolete files)
(deleted),
application/xhtml+xml
|
Details | |
(deleted),
text/xml
|
Details | |
(deleted),
text/xml
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
vlad
:
review+
vlad
:
approval1.9+
|
Details | Diff | Splinter Review |
Mac native form controls seem to draw at incorrect positions in offscreen surfaces: perhaps they are off by the difference between the window origin and the surface origin. (See also bug 382048 for buttons.)
Steps to reproduce: load attached testcase
Expected results: the form controls are placed correctly relative to the text associated with them, and they handle events resulting from clicking on them
Actual results: the form controls are placed too far down and too far to the right, but they are activated by clicking where they should be
Flags: blocking1.9?
Assignee: joshmoz → nobody
Component: Widget: Cocoa → GFX: Thebes
Flags: blocking1.9? → blocking1.9+
QA Contact: cocoa → thebes
Assignee | ||
Comment 1•18 years ago
|
||
Assignee | ||
Comment 2•18 years ago
|
||
Assignee | ||
Comment 3•18 years ago
|
||
Assignee | ||
Updated•18 years ago
|
Assignee: nobody → jdaggett
Assignee | ||
Comment 4•18 years ago
|
||
Related issues: 382048, 382049, 382050
Assignee | ||
Comment 5•18 years ago
|
||
Note the big white box that appears, the drawing operation is shifted over and down 8 pixels.
Assignee | ||
Comment 6•17 years ago
|
||
This patch fixes up the offset problem but the testcase is still not drawing correctly. For some reason, when an offscreen is used (via PushGroup / PopGroupToSource in nsDisplayOpacity::Paint) the resulting copy to the window context is not drawn correctly. In the "trivial testcase with padding added" you see a white box where the radio button should appear. In fact, *any* kind of drawing between the PushGroup / PopGroup appears as white, no matter what the color / alpha settings. I'm guessing the problem lies in the quartz surface code, within the code that paints the offscreen into the window context, but that's just a guess at this point.
Reporter | ||
Comment 7•17 years ago
|
||
That looks like the complete file, not a diff.
Please include the fixes to the reftest manifest to mark the relevant tests not failing/random in the patch; otherwise when it's checked in the tree will go orange.
Assignee | ||
Comment 8•17 years ago
|
||
let's try that again...
Attachment #274890 -
Attachment is obsolete: true
Assignee | ||
Comment 9•17 years ago
|
||
Note: the patch fixes the offset problem but not the other related drawing problems (see comment #6). So some things will render correctly, some will still not. See bug 372994 for a more complete list.
Status: NEW → ASSIGNED
Assignee | ||
Comment 10•17 years ago
|
||
Finally figured out the root cause of this - we're setting up an offscreen CGBitmapContext with a default composite operation of "copy". Setting this to "source over" looks like it fixes the problem. Still working on what the appropriate way to fix this is, probably something like setting the default composite operation to the default operation obtained from the existing surface.
One thing that sucks is that we're using a private Quartz call, CGContextSetCompositeOperation, it would be nice if we could avoid having to use this, but that may not be possible.
So in summary, there are two subproblems here:
(1) offsets calculated incorrectly (fixed by "actual patch")
(2) default composite operation is set to "copy" not "source over"
Ah! I didn't understand this at first, but now it makes sense. So the fix should be to, at the end of _clone_similar, to set the composite operation back to SrcOver (Copy should still be used for the DrawImage call). I guess OSX ops assume that the current operation is SrcOver... or rather, more likely, they just use whatever's there.
So actually, a better fix would be to explicitly set it to SrcOver at the start of the native theme drawing code (and don't bother setting it to SrcOver in clone()) -- the quartz surface doesn't care what the current composite op is, it sets it to whatever it should be for every operation. (We'd run into this same problem if, e.g., a native theme bit was drawn at some point after a SVG file that set some non-OVER composite op.) But if the native theme code expects it to be OVER, so it should ensure that that's the case.
Assignee | ||
Comment 12•17 years ago
|
||
Based on Vlad's suggestion, just fix this within the Mac theme code:
(1) calculate the offset correctly
(2) set the composite operation to "sourceOver" via private Quartz call
This managed to fix a reftest failure bug, bug 377118.
Attachment #275043 -
Attachment is obsolete: true
Attachment #275568 -
Flags: review?(vladimir)
Assignee | ||
Comment 13•17 years ago
|
||
Note: the composite operation change will have no effect outside of nsNativeThemeCocoa::DrawWidgetBackground, since it takes place within a gstate save/restore pair.
Comment on attachment 275568 [details] [diff] [review]
full patch with reftest list fixup
Looks good! I can get this checked in for you shortly.
Attachment #275568 -
Flags: review?(vladimir)
Attachment #275568 -
Flags: review+
Attachment #275568 -
Flags: approval1.9+
Checked in, though the unit test tinderbox is backed up.. but other mac tinderboxes are green.
Assignee | ||
Comment 16•17 years ago
|
||
Fixed in latest nightly:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a8pre) Gecko/2007080804 Minefield/3.0a8pre
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•