Closed Bug 970294 Opened 7 years ago Closed 7 years ago

Perspective + Box-shadow leads to incorrect box size model when transform-ing


(Core :: Graphics: Layers, defect)

Not set



Tracking Status
firefox27 --- unaffected
firefox28 + fixed
firefox29 + fixed
firefox30 + fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed


(Reporter: bugzilla, Assigned: mattwoodrow)



(Keywords: regression, Whiteboard: [good first verify])


(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20140205162153

Steps to reproduce:

Created an fixed size object with these css properties applied :
- ouset box-shadow (either one of 4 props set)
- perspective
- transform3D with deg != 0%180

Test case here :

Actual results:

Object is transformed as if its size was increased by the shadow's pixels

Expected results:

Object should transform using it's own size, and shadow should not interfere.


1/ If the angle is multiple of 180, this doesn't happen. Therefore, an animation/transition using this combination of attributes appears flickering (for example :

2/ This is a regression : was OK in Firefox 27.
Antwan, what part of the testcase is the part I should be looking at here?

What I see is that the behavior of the third div in the "perspective" set seems to depend on window width, and in particular on the line-wrapping(?).
The 3rd element of the 2nd line should be positioned like the 1st element of the second line.
OK.  So for what it's worth, it doesn't seem to be in Firefox 27, for me...
I loaded the testcase in Firefox 26RC, 27RC, 28beta3, Aurora 29, Nightly 30, Internet Explorer and all browsers display the same result. Am I missing something?
Flags: needinfo?(antwan)
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #4)
> I loaded the testcase in Firefox 26RC, 27RC, 28beta3, Aurora 29, Nightly 30,
> Internet Explorer and all browsers display the same result.
The result that I`m talking about can be seen in this screenshot
Was happening only when hardware acceleration was disabled.
Try it on Firefox 27 vs Firefox 28 with both layers.acceleration.disabled:true.

As this setting is not defaulted, I'm not sure it's worth further investigation.
Closed: 7 years ago
Flags: needinfo?(antwan)
Resolution: --- → INVALID
Hardware accel can get disabled for all sorts of reasons....
Component: Layout: View Rendering → Graphics
Ever confirmed: true
Resolution: INVALID → ---
Ok so with hardware acceleration off I was able to reproduce the issue and got this regression range:

Last good revision: 53d55d2d0a25 (2013-11-26)
First bad revision: 6ecf0c4dfcbe (2013-11-27)

Last good revision: c9ff42335f8a
First bad revision: 35014d18ff1f

If you need any more assistance from QA please let me know.
OS: Windows 7 → All
Hardware: x86_64 → All
Version: 28 Branch → Trunk
Bogdan, that's awesome, thank you!
Blocks: 932888
Component: Graphics → Graphics: Layers
Flags: needinfo?(ncameron)
I expect this is due to a bug in Azure somewhere - i.e., previously we were using Thebes for drawing and pot-bug 932888 we use Azure for that and so it goes wrong. Passing over to mattwoodrow who knows all about Azure vs Thebes and the basic layer manager.
Flags: needinfo?(ncameron) → needinfo?(matt.woodrow)
I see what the bug is, caused by

Previously we set a device offset on the intermediate surface, with Azure we instead set a transform on the context.

Unfortunately BasicLayerManager::PaintLayer can clobber the transform (but not the device offset of a gfxASurface), so this gets lost.

I'll have a think about the best way to fix this.
Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)
This way the users of the gfxContext can't clobber our device offset.
Attachment #8383220 - Flags: review?(ncameron)
Comment on attachment 8383220 [details] [diff] [review]
Use gfxContext's device offset functionality

Review of attachment 8383220 [details] [diff] [review]:

::: gfx/layers/basic/BasicLayerManager.cpp
@@ +959,5 @@
>      if (!untransformedDT) {
>        return;
>      }
> +    nsRefPtr<gfxContext> groupTarget = new gfxContext(untransformedDT, Point(bounds.x, bounds.y));

nit: long line

::: gfx/thebes/gfxContext.h
@@ +50,5 @@
>       * Initialize this context from a DrawTarget.
>       * Strips any transform from aTarget.
>       * aTarget will be flushed in the gfxContext's destructor.
>       */
> +    gfxContext(mozilla::gfx::DrawTarget *aTarget, const mozilla::gfx::Point& aDeviceOffset = mozilla::gfx::Point());

nit: long line
Attachment #8383220 - Flags: review?(ncameron) → review+
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Comment on attachment 8383220 [details] [diff] [review]
Use gfxContext's device offset functionality

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 932888.
User impact if declined: Incorrect 3d transform rendering without hardware acceleration.
Testing completed (on m-c, etc.): Been on m-c for a week, confirmed testcase 
Risk to taking this patch (and alternatives if risky): Low risk, just changes a transform matrix.
String or IDL/UUID changes made by this patch: None
Attachment #8383220 - Flags: approval-mozilla-beta?
Attachment #8383220 - Flags: approval-mozilla-aurora?
Attachment #8383220 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [good first verify]
Attachment #8383220 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 978492
Blocks: 964688
You need to log in before you can comment on or make changes to this bug.