3d css does not work correctly with iframes

RESOLVED FIXED in mozilla10

Status

()

Core
Layout: View Rendering
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Gregg Tavares, Assigned: mattwoodrow)

Tracking

({testcase})

Trunk
mozilla10
x86_64
Linux
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox10-)

Details

(Whiteboard: [inbound])

Attachments

(4 attachments, 4 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 564624 [details]
interactive-3d-css-element.html

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111004 Firefox/10.0a1
Build ID: 20111004030858

Steps to reproduce:

FF version: 10.0a1 (2011-10-04)

Visit this page 
http://greggman.com/downloads/examples/interactive-3d-css-element.html
or use the attached file


Actual results:

The iframe content is not interactive.


Expected results:

You should be able to click links, type in search boxes, etc.

Note: I know 3d css is brand new to Firefox. I just thought you'd find it useful to have a simple example.

Works in Firefox
Attachment #564624 - Attachment mime type: text/plain → text/html
Component: General → Layout
Keywords: testcase
Product: Firefox → Core
QA Contact: general → layout
Version: 10 Branch → Trunk
The attached testcase worksforme on Mac.  I can click links, type in the search box, etc.

Is the issue Linux-only?
Component: Layout → Layout: View Rendering
QA Contact: layout → layout.view-rendering
It doesn't work for me on Linux, either in a debug build or in the 2011-10-07-03-12-27-mozilla-central x86_64 nightly.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hmm.  Testing it some more on Mac, I can click on the left sides of links and the textbox and things work, but if I click on their right sides the click is not registered.

(I also tried turning off hardware acceleration, and while that made the transformed content all jaggy it had no effect on the click behavior.)
Yeah, I just realized the same thing -- if I make the window narrower, I can interact with stuff on the left side of the page -- and as I narrow the window further, I can interact with more.  So it seems like there's one test that's off somehow.
Ah, ok.  My window is normally about 900px wide, which probably counts as "narrow"...
tracking-firefox10: --- → ?
Sounds like a bug in event handling logic
Assignee: nobody → matt.woodrow
(Assignee)

Comment 7

6 years ago
Thanks for filing this.

I notice that if I resize my window so that the left edge of the page works, and the right edge doesn't, I can start selecting text from the left edge and drag into the right side and get accurate text selection.

Anyone have any ideas what different code paths are taken that could mean that the mouse point during text selection is correct, but wrong for mouse-over/click events? Would be nice to narrow down my search.

I'll take a proper look at this on Monday.
Setting gDumpEventList to true in nsLayoutUtils.cpp may be a reasonable way to start debugging.  When I did that briefly this morning it seemed (at a quick glance -- could be wrong) like the decision on whether to *build* the display list inside the iframe was incorrect, but if we decided to build the display list inside the iframe, then the display list inside it was constructed correctly and its HitTest method worked correctly.
(Assignee)

Comment 9

6 years ago
So at:

http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp#6579

We are calling TransformRect with nsPoint(0, 0) instead of the actual offset of the frame and are under the assumption that this doesn't affect the overflow calculation.

This is not true with perspective transformations, and we are getting an overflow rect that is too small.

Any suggestions on how we can get the actual offset here? Is it even a constant for a frame?
The input overflow rect is relative to the top-left of the frame's pre-transform border-box. The output overflow rect is relative to (mRect.x, mRect.y) in the parent frame's coordinate space. Does that help?
(Assignee)

Comment 11

6 years ago
I don't believe so.

I think we just need to check ToReferenceFrame(), assuming that value will be correct at this point.
There's no ToReferenceFrame, because there's no display list. The reference frame is somewhat arbitrary anyway.
(Assignee)

Comment 13

6 years ago
The offset passed here actually changes the size (not just x/y) of the returned overflow.
So what coordinate space do you want the coordinates in?  Does it need to be relative to the -moz-transform-origin of the frame with the transform or something like that?
(Assignee)

Comment 15

6 years ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #13)
> The offset passed here actually changes the size (not just x/y) of the
> returned overflow.

This makes no sense at all. It only changes it if you don't adjust the coordinate system of the passed in rectangle too. Must have been half asleep.

I think a more relevant problem is that mRect = {0, 0, 0, 0} and GetParent()->mRect = {0, 0, 0, 0}.

When it comes to drawing time, the parent frame rect has an offset of 12480, 9600.

Since computing the transform for a preserve-3d child gives you a transform that changes to the coordinate space of the rootmost parent of the chain, then we need these offsets.
(In reply to Matt Woodrow (:mattwoodrow) from comment #15)
> I think a more relevant problem is that mRect = {0, 0, 0, 0} and
> GetParent()->mRect = {0, 0, 0, 0}.

That sounds OK.

> When it comes to drawing time, the parent frame rect has an offset of 12480,
> 9600.

You mean its offset to the reference frame is 12480,9600? That's possible since the reference frame is generally the root frame of the topmost document.

> Since computing the transform for a preserve-3d child gives you a transform
> that changes to the coordinate space of the rootmost parent of the chain,
> then we need these offsets.

You mean the first ancestor that's not preserve-3d? I guess you'll need to find that explicitly.
(Assignee)

Comment 17

6 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #16)
> > When it comes to drawing time, the parent frame rect has an offset of 12480,
> > 9600.
> 
> You mean its offset to the reference frame is 12480,9600? That's possible
> since the reference frame is generally the root frame of the topmost
> document.

No I mean GetParent->mRect = { 12480, 9600, width, height }

Since these are different between computing the overflow, and drawing, we get different matrices.

> 
> > Since computing the transform for a preserve-3d child gives you a transform
> > that changes to the coordinate space of the rootmost parent of the chain,
> > then we need these offsets.
> 
> You mean the first ancestor that's not preserve-3d? I guess you'll need to
> find that explicitly.

Yes, I did.
(In reply to Matt Woodrow (:mattwoodrow) from comment #17)
> No I mean GetParent->mRect = { 12480, 9600, width, height }
> 
> Since these are different between computing the overflow, and drawing, we
> get different matrices.

Ah right. Because when we reflow the child we haven't finished reflowing all the ancestors yet. This is a problem.

When we finish reflowing the root of a preserve-3d subtree, we probably need to traverse the preserve-3d descendants and fix up their overflow areas :-(.

This might depend on bug 524925, but I hope not.
(Assignee)

Comment 19

6 years ago
Created attachment 566430 [details] [diff] [review]
WIP

Once we get to storing the overflowing for the parent of a preserve-3d hierarchy, fix up the overflow areas of any children.

This seems to get reasonable sized overflow rects set, but it's still at the wrong positions. Both the untransformed overflow, and frame size should be in the same (correct) coordinate space right?
Attachment #566430 - Flags: feedback?(roc)
+        desiredSize.ScrollableOverflow() = child->GetVisualOverflowRectRelativeToSelf();

GetScrollableOverflowRectRelativeToSelf()?

So we're calling FinishAndStoreOverflow on a preserve-3d child twice, and we're applying transforms *both times* right? That seems wrong. Seems like you should get the pre-transform overflow area here before you call FinishAndStoreOverflow again?

(In reply to Matt Woodrow (:mattwoodrow) from comment #19)
> This seems to get reasonable sized overflow rects set, but it's still at the
> wrong positions. Both the untransformed overflow, and frame size should be
> in the same (correct) coordinate space right?

Not sure what you mean.
(Assignee)

Comment 21

6 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #20)
> +        desiredSize.ScrollableOverflow() =
> child->GetVisualOverflowRectRelativeToSelf();
> 
> GetScrollableOverflowRectRelativeToSelf()?

This doesn't exist yet, but I plan to add it.

> 
> So we're calling FinishAndStoreOverflow on a preserve-3d child twice, and
> we're applying transforms *both times* right? That seems wrong. Seems like
> you should get the pre-transform overflow area here before you call
> FinishAndStoreOverflow again?

GetVisualOverflowRectRelativeToSelf is the pre-transformed overflow rect isn't it?

> 
> (In reply to Matt Woodrow (:mattwoodrow) from comment #19)
> > This seems to get reasonable sized overflow rects set, but it's still at the
> > wrong positions. Both the untransformed overflow, and frame size should be
> > in the same (correct) coordinate space right?
> 
> Not sure what you mean.

We're unioning the frame size and the result of GetVisualOverflowRectRelativeToSelf (pre-transform overflow rect). I believe that these are in the same coordinate space.
(In reply to Matt Woodrow (:mattwoodrow) from comment #21)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #20)
> > So we're calling FinishAndStoreOverflow on a preserve-3d child twice, and
> > we're applying transforms *both times* right? That seems wrong. Seems like
> > you should get the pre-transform overflow area here before you call
> > FinishAndStoreOverflow again?
> 
> GetVisualOverflowRectRelativeToSelf is the pre-transformed overflow rect
> isn't it?

Er, right.

> > (In reply to Matt Woodrow (:mattwoodrow) from comment #19)
> > > This seems to get reasonable sized overflow rects set, but it's still at the
> > > wrong positions. Both the untransformed overflow, and frame size should be
> > > in the same (correct) coordinate space right?
> > 
> > Not sure what you mean.
> 
> We're unioning the frame size and the result of
> GetVisualOverflowRectRelativeToSelf (pre-transform overflow rect). I believe
> that these are in the same coordinate space.

Yes.

Actually, passing the pre-transform overflow area as the input to FinishAndStoreOverflow is still wrong since there's still a bunch of processing that will get repeated, such as filter effects. What you really need to do is save the overflow area that was input to FinishAndStoreOverflow the first time and repeat that input again.
(Assignee)

Updated

6 years ago
Blocks: 692968
(Assignee)

Comment 23

6 years ago
Created attachment 566728 [details] [diff] [review]
Reduced test case

I've reduced the test case a fair bit, and it's still broken even with my current patch.

The problem appears to be the margin: 0 auto; set on the intermediate frame.

The problem case is approximately:

<div id="parent">
  <div id="one" style="-moz-transform-style:preserve-3d;">
    <div id="two" style="-moz-transform-style:preserve-3d; margin: auto;">
      <div id="three" style="-moz-transform: <transforms>;"></div>
    </div>
  </div>
</div>

When we calculate the overflow area for three, we convert to the coordinate space of parent (instead of two as we normally would without preserve-3d). The overflow rects for parent, one and two just copy this overflow area (and combine it with any other children they may have) since it's already in the 'right' coordinate space.

For some reason this isn't taking the margin shift (around 200pixels?) into account and our final overflow area is positioned incorrectly. If I remove the margin, then this testcase works.

Any ideas where I should be looking to compensate for this margin?
(Assignee)

Comment 24

6 years ago
Created attachment 566729 [details] [diff] [review]
WIP v2
Attachment #566430 - Attachment is obsolete: true
Attachment #566430 - Flags: feedback?(roc)
Attachment #566729 - Flags: feedback?(tnikkel)
(Assignee)

Updated

6 years ago
Attachment #566729 - Flags: feedback?(roc)
(In reply to Matt Woodrow (:mattwoodrow) from comment #23)
> The problem case is approximately:
> 
> <div id="parent">
>   <div id="one" style="-moz-transform-style:preserve-3d;">
>     <div id="two" style="-moz-transform-style:preserve-3d; margin: auto;">
>       <div id="three" style="-moz-transform: <transforms>;"></div>
>     </div>
>   </div>
> </div>
> 
> When we calculate the overflow area for three, we convert to the coordinate
> space of parent (instead of two as we normally would without preserve-3d).
> The overflow rects for parent, one and two just copy this overflow area (and
> combine it with any other children they may have) since it's already in the
> 'right' coordinate space.

The overflow rect contribution of "three" needs to be adjusted to account for the x/y offset introduced by the auto margin.
(Assignee)

Comment 26

6 years ago
Created attachment 566929 [details] [diff] [review]
Recompute Preserve-3d children overflow rects after all needed sizes are available

This fixes the test case for me!
Attachment #566729 - Attachment is obsolete: true
Attachment #566729 - Flags: feedback?(tnikkel)
Attachment #566729 - Flags: feedback?(roc)
Attachment #566929 - Flags: review?(roc)
Comment on attachment 566929 [details] [diff] [review]
Recompute Preserve-3d children overflow rects after all needed sizes are available

Review of attachment 566929 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with that fixed.

But we desperately need tests here.

::: layout/generic/nsFrame.cpp
@@ +6499,5 @@
> +    Properties().Set(nsIFrame::InitialOverflowProperty(),
> +                     new nsOverflowAreas(aOverflowAreas));
> +  } else if (initial != &aOverflowAreas) {
> +    *initial = aOverflowAreas;
> +  }

We cannot add this property to every frame, it would kill us performance-wise.

I think we can add it only for preserves-3d children, and only for the children where the overflow area isn't exactly aNewSize. In which case, we should carefully document when it's present and when it isn't.
Attachment #566929 - Flags: review?(roc) → review+
(Assignee)

Comment 28

6 years ago
Created attachment 567043 [details] [diff] [review]
Recompute Preserve-3d children overflow rects after all needed sizes are available v2

Fixed review comments, carrying forward r=roc
Attachment #566929 - Attachment is obsolete: true
Attachment #567043 - Flags: review+
(Assignee)

Comment 29

6 years ago
Created attachment 567044 [details] [diff] [review]
Preserve-3d test with margin
Attachment #567044 - Flags: review?(roc)
(Assignee)

Comment 30

6 years ago
Created attachment 567054 [details] [diff] [review]
Preserve-3d test with margin v2

Now with more tests :)
Attachment #567044 - Attachment is obsolete: true
Attachment #567044 - Flags: review?(roc)
Attachment #567054 - Flags: review?(roc)
Attachment #567054 - Flags: review?(roc) → review+
(Assignee)

Comment 31

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ecd9735b523
https://hg.mozilla.org/integration/mozilla-inbound/rev/16bd56ed52ee
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/4ecd9735b523
https://hg.mozilla.org/mozilla-central/rev/16bd56ed52ee
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10

Updated

5 years ago
tracking-firefox10: ? → -

Updated

5 years ago
Depends on: 723991
You need to log in before you can comment on or make changes to this bug.