Closed
Bug 654950
Opened 10 years ago
Closed 10 years ago
[HW-Layers] when using a transparent DIV, and acceleration enabled google maps marker are invisible
Categories
(Core :: Graphics, defect)
Tracking
()
VERIFIED
FIXED
mozilla6
People
(Reporter: lorife, Assigned: roc)
References
()
Details
(Keywords: regression)
Attachments
(3 files, 2 obsolete files)
User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1 Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1 If you click on the link and then on "dove siamo" you'll see that if acceleration hardware is enabled and the div is transparent the marker in google maps is invisible. If i disable the acceleration i can see the marker again Reproducible: Always Steps to Reproduce: 1.enable acceleration 2.create a transparent div 3.put a google map and a marker in it 4.marker is invisible Actual Results: i can't see the marker in google maps Expected Results: i should have seen the marker graphic card: nvidia geforce 6600
![]() |
||
Comment 1•10 years ago
|
||
Confirmed on http://hg.mozilla.org/mozilla-central/rev/c3c4c902e9cd Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110504 Firefox/6.0a1 ID:20110504030604 Graphics Adapter Description: ATI Radeon HD 4300/4500 Series Vendor ID: 1002 Device ID: 954f Adapter RAM: 512 Adapter Drivers: aticfx64 aticfx64 aticfx32 aticfx32 atiumd64 atidxx64 atiumdag atidxx32 atiumdva atiumd6a atitmm64 Driver Version: 8.841.0.0 Driver Date: 4-5-2011 Direct2D Enabled: true DirectWrite Enabled: true (6.1.7601.17563, font cache n/a) WebGL Renderer: Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.611) GPU Accelerated Windows: 1/1 Direct3D 10 It works if set layers.acceleration.disabled t o true.
Status: UNCONFIRMED → NEW
Component: Preferences → Graphics
Ever confirmed: true
Product: Firefox → Core
QA Contact: preferences → thebes
![]() |
||
Comment 2•10 years ago
|
||
There are 2 regression with enabled layers.acceleration. #1 Regression window: Works: http://hg.mozilla.org/mozilla-central/rev/af89c96d0939 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101107 Firefox/4.0b8pre ID:20101108010423 Fails but appears after drag scroll: http://hg.mozilla.org/mozilla-central/rev/f35c89eac392 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101108 Firefox/4.0b8pre ID:20101108023254 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=af89c96d0939&tochange=f35c89eac392 #2 Regression windows: Fails but it appears after drag scroll: http://hg.mozilla.org/mozilla-central/rev/d538a677628e Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101221 Firefox/4.0b9pre ID:20101221094054 Fails: http://hg.mozilla.org/mozilla-central/rev/9b02aa0de19c Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101221 Firefox/4.0b9pre ID:20101221105418 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d538a677628e&tochange=9b02aa0de19c
Keywords: regression
![]() |
||
Updated•10 years ago
|
Summary: when using a transparent DIV, and acceleration enabled google maps marker are invisible → [D3D9/10] when using a transparent DIV, and acceleration enabled google maps marker are invisible
Version: unspecified → Trunk
![]() |
||
Comment 3•10 years ago
|
||
roc, any ideas? Your stuff seems to be most of that first regression range.
![]() |
||
Comment 4•10 years ago
|
||
#1 Regression build from f35c89eac392 : fails but appears after drag scroll build from 362567eee982 : works build from 5d82a6476d53 : works build from d01932e9114e : works build from 33652dc3d274 : works build from 75ca0ab361b8 : works Triggered by: f35c89eac392 Robert O'Callahan — Bug 602200. Share code to compute effective transforms and opacity, and snap effective transforms. r=bas,sr=vlad,a=blocker #2 Regression build from ab173acf8a1f : fails build from d538a677628e : fails but appears after drag scroll Triggered by: ab173acf8a1f Oleg Romashin — Bug 607653 - avoid temporary fbos/textures on transformed layers, when possible. r=roc a=approval2.0
Assignee | ||
Comment 5•10 years ago
|
||
The link is dead.
(In reply to comment #5) > The link is dead. sorry i released it, i changed the link: www.rtshow.it
by the way, i have this problem also on another pc with an ati hd 4850, if acceleration is enabled in mozilla firefox 4.0.1
Assignee | ||
Comment 8•10 years ago
|
||
It works for me, but I have a large set of patches applied...
Assignee | ||
Comment 9•10 years ago
|
||
In particular I have patches for bus 648483, 647560, 650228, 637852 and 648277 in my tree. So let's retest once some of those are fixed...
Assignee | ||
Comment 10•10 years ago
|
||
Oops. My configuration was wrong, and I still see this bug with all those patches.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → roc
Assignee | ||
Comment 11•10 years ago
|
||
When I save the testcase locally, the marker doesn't work at all. So making a reduced testcase seems hard :-(. lorife, do you have a testcase other than your entire site? :-)
Reporter | ||
Comment 12•10 years ago
|
||
Reporter | ||
Comment 13•10 years ago
|
||
I attached a little test. I don't know why now it's half-marker..but still there's some problem. Please forgive me if it's indented wrong..i copy-pasted the important parts.
![]() |
||
Comment 14•10 years ago
|
||
Extract to local HDD. Open "RT Show.htm".
![]() |
||
Updated•10 years ago
|
Attachment #533236 -
Attachment is obsolete: true
![]() |
||
Comment 15•10 years ago
|
||
![]() |
||
Comment 16•10 years ago
|
||
Red and blue rectangles should appear at the left-top corner.
Attachment #533255 -
Attachment is obsolete: true
Comment 17•10 years ago
|
||
This occurs on GL layers too.
Summary: [D3D9/10] when using a transparent DIV, and acceleration enabled google maps marker are invisible → [HW-Layers] when using a transparent DIV, and acceleration enabled google maps marker are invisible
Assignee | ||
Comment 18•10 years ago
|
||
(In reply to comment #16) > Created attachment 533269 [details] > Semi-reduced html > > Red and blue rectangles should appear at the left-top corner. Amazing work Alice!
Assignee | ||
Comment 19•10 years ago
|
||
The problem here is in Layer::CalculateScissorRect: if (aIntermediate) { scissorRect.MoveBy(- aVisibleRect.TopLeft()); } else if (clipRect) { scissorRect.IntersectRect(scissorRect, aParentScissor); } When a container with an intermediate surface (aIntermediate true) has an immediate child with a cliprect, we correctly adjust the scissorRect to be relative to its intermediate surface (whose top-left is aVisibleRect.TopLeft()). But when that container has a grandchild with a cliprect, the parent container layer's call to CalculateScissorRect does not adjust for the grandparent's intermediate surface top-left.
Assignee | ||
Comment 20•10 years ago
|
||
Attachment #533529 -
Flags: review?
Assignee | ||
Updated•10 years ago
|
Attachment #533529 -
Flags: review? → review?(bas.schouten)
Comment 21•10 years ago
|
||
Comment on attachment 533529 [details] [diff] [review] fix Review of attachment 533529 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/Layers.cpp @@ +309,5 @@ > } > > nsIntRect > +Layer::CalculateScissorRect(const nsIntRect& aCurrentScissorRect, > + const gfxMatrix* aWorldTransform) Any reason we shouldn't do const gfxMatrix &aWorldTransform? I hate the name aWorldTransform actually, would've preferred something like GlobalTransform but oh well, this patch isn't the place to change it. @@ +316,5 @@ > + NS_ASSERTION(container, "This can't be called on the root!"); > + > + // Establish initial clip rect: it's either the one passed in, or > + // if the parent has an intermediate surface, it's the extents of that surface. > + nsIntRect result; I think we should rename 'result' to something like 'existingClip' or something like that to make it more obvious throughout the rest of this function what it does. @@ +347,5 @@ > + // Find the nearest ancestor with an intermediate surface > + do { > + container = container->GetParent(); > + } while (container && !container->UseIntermediateSurface()); > + } How about we move this code into the 'else' clause of the previous if statement (i.e. container->UseIntermediateSurface()). We declare clipRect before the first if statement and move if (clipRect->IsEmpty()) etc. into the else clause as well. In the container->UseIntermediateSurface we'll move it, but that's fine (it'll still be empty). The only downside would be you'd need to duplicate the if (!clipRect) statement into both branches but I personally think it'll increase the readability a bit. I'll leave it up to you whether to do this or not.
Attachment #533529 -
Flags: review?(bas.schouten) → review+
Assignee | ||
Comment 22•10 years ago
|
||
(In reply to comment #21) > ::: gfx/layers/Layers.cpp > @@ +309,5 @@ > > } > > > > nsIntRect > > +Layer::CalculateScissorRect(const nsIntRect& aCurrentScissorRect, > > + const gfxMatrix* aWorldTransform) > > Any reason we shouldn't do const gfxMatrix &aWorldTransform? Yes, then we'd have to construct an identity matrix and pass it in from the D3D9 callers. This is slightly more concise and efficient. > I hate the name > aWorldTransform actually, would've preferred something like GlobalTransform > but oh well, this patch isn't the place to change it. Yeah, this is consistent with the GL code that's the only caller. > @@ +316,5 @@ > > + NS_ASSERTION(container, "This can't be called on the root!"); > > + > > + // Establish initial clip rect: it's either the one passed in, or > > + // if the parent has an intermediate surface, it's the extents of that surface. > > + nsIntRect result; > > I think we should rename 'result' to something like 'existingClip' or > something like that to make it more obvious throughout the rest of this > function what it does. OK. > @@ +347,5 @@ > > + // Find the nearest ancestor with an intermediate surface > > + do { > > + container = container->GetParent(); > > + } while (container && !container->UseIntermediateSurface()); > > + } > > How about we move this code into the 'else' clause of the previous if > statement (i.e. container->UseIntermediateSurface()). We declare clipRect > before the first if statement and move if (clipRect->IsEmpty()) etc. into > the else clause as well. OK. > In the container->UseIntermediateSurface we'll move it, but that's fine > (it'll still be empty). > > The only downside would be you'd need to duplicate the if (!clipRect) > statement into both branches but I personally think it'll increase the > readability a bit. OK
Assignee | ||
Updated•10 years ago
|
Whiteboard: [needs landing]
Assignee | ||
Comment 23•10 years ago
|
||
(In reply to comment #21) > How about we move this code into the 'else' clause of the previous if > statement (i.e. container->UseIntermediateSurface()). We declare clipRect > before the first if statement and move if (clipRect->IsEmpty()) etc. into > the else clause as well. > > In the container->UseIntermediateSurface we'll move it, but that's fine > (it'll still be empty). > > The only downside would be you'd need to duplicate the if (!clipRect) > statement into both branches but I personally think it'll increase the > readability a bit. > > I'll leave it up to you whether to do this or not. I tried this, and it got ugly. 'scissor' needs to be defined before the first 'if' too, and then set to *clipRect in both branches. There's too much duplicated code.
Comment 24•10 years ago
|
||
(In reply to comment #23) > (In reply to comment #21) > > How about we move this code into the 'else' clause of the previous if > > statement (i.e. container->UseIntermediateSurface()). We declare clipRect > > before the first if statement and move if (clipRect->IsEmpty()) etc. into > > the else clause as well. > > > > In the container->UseIntermediateSurface we'll move it, but that's fine > > (it'll still be empty). > > > > The only downside would be you'd need to duplicate the if (!clipRect) > > statement into both branches but I personally think it'll increase the > > readability a bit. > > > > I'll leave it up to you whether to do this or not. > > I tried this, and it got ugly. 'scissor' needs to be defined before the > first 'if' too, and then set to *clipRect in both branches. There's too much > duplicated code. Oh, I thought scissor could just be set before the first 'if' too. But it's not that important.
Comment 25•10 years ago
|
||
(In reply to comment #24) > (In reply to comment #23) > > (In reply to comment #21) > > I tried this, and it got ugly. 'scissor' needs to be defined before the > > first 'if' too, and then set to *clipRect in both branches. There's too much > > duplicated code. > > Oh, I thought scissor could just be set before the first 'if' too. > > But it's not that important. Never mind, of course it can't. But why does it need to be defined before the first 'if'?
Assignee | ||
Comment 26•10 years ago
|
||
Because we use it in both the UseIntermediateSurface() and !UseIntermediateSurface() cases.
Comment 27•10 years ago
|
||
(In reply to comment #26) > Because we use it in both the UseIntermediateSurface() and > !UseIntermediateSurface() cases. But only after the if (container) clause which could completely go after the first if/else statement right?
Assignee | ||
Comment 28•10 years ago
|
||
I've lost track of what the code would look like. Let's not worry about it.
Assignee | ||
Comment 29•10 years ago
|
||
http://hg.mozilla.org/projects/cedar/rev/ed0850a1bbb7
Flags: in-testsuite+
Whiteboard: [needs landing] → [fixed-in-cedar]
Assignee | ||
Comment 30•10 years ago
|
||
The test fails on Mac with GL because GL and Quartz alpha computation differs slightly. So I disabled it there. http://hg.mozilla.org/projects/cedar/rev/54134ae42f5d
Comment 31•10 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/ed0850a1bbb7 http://hg.mozilla.org/mozilla-central/rev/54134ae42f5d
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-cedar]
Target Milestone: --- → mozilla6
Assignee | ||
Comment 32•10 years ago
|
||
I'm not sure whether we should try to get this fix into Firefox 5. We probably should at least give it a little bake time on trunk/Aurora. It doesn't have any known duplicates so maybe this case is rare enough fixing it in FF6 is good enough. Thanks again Alice!
Comment 33•10 years ago
|
||
Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0b3 Considering comment16, the two rectangles are present.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•