Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Need to be able to specify different clips for the same scrolling root and let items pick which one they want #840

Closed
mstange opened this issue Feb 7, 2017 · 12 comments
Closed

Need to be able to specify different clips for the same scrolling root and let items pick which one they want #840

mstange opened this issue Feb 7, 2017 · 12 comments
Assignees
@mrobinson
Labels
bugzilled

Comments

@mstange
Copy link
Contributor

mstange commented Feb 7, 2017

If clips are going to be set on scrolling roots, then we're going to have a problem when we have two different items that are scrolled by the same scrolling root but that should be clipped to different (unscrolled) clips.

In Gecko we have this case for the caret in textboxes. (This special case is a little unfortunate but it seems a lot easier and less risky to adjust the webrender API for this case than to change how Gecko layout positions the caret or adds extra space after the last glyph in a textbox.)

Basically, a textbox is a scroll frame which has a bit of padding. The text inside the textbox is clipped to the content-box of the frame, and the caret is clipped to a rectangle that's a few pixels larger than the content-box. Both these clips stay in place when the textbox is scrolled.

We need a way to represent this with webrender.
In Gecko, we don't have clips on SRs - instead, each display item has a list of clips, and each of the clip items in that stack is annotated with the SR that moves this clip.
The other way to address this in webrender, which would be closer to the plan that has been proposed, would be to allow multiple SR nodes in the SRF tree for each SR, so that you can specify a different clip on each node but have them both refer to the same scroll offset. And we'd need to come up with different names for two concepts: the SR itself, and the (SR, clip) pair that forms the node in the tree.

cc @mrobinson @kvark @jrmuizel
@mrobinson mrobinson self-assigned this Feb 7, 2017
@mrobinson
Copy link
Member

Internally clipping and positioning (from scroll positions) will soon be completely independent in WebRender, so I think the only difficult question here is what is the best way to represent this in the API.

@mstange
Copy link
Contributor Author

mstange commented Feb 7, 2017

Oh, so the webrender API for this hasn't actually been decided on yet. Good!

@glennw
Copy link
Member

glennw commented Mar 28, 2017

@mrobinson I think this can be closed now?

@mrobinson
Copy link
Member

@glennw I've actually been exploring using WebRender clipping to handle CSS clip in Servo, and I think we'll need one more API change to deal with this. If I understand correctly, we'll need to position and clip using the scrolling root and then also clip with another entirely separate clip (which might depend on a different scrolling position).

@glennw
Copy link
Member

glennw commented Mar 28, 2017

OK, that doesn't sound like it will be a major API change?

@mrobinson
Copy link
Member

@glennw I think we just need to add another clip field do every display list item. The first field will be positioning + clipping and the other will be scroll root independent clipping.

@glennw
Copy link
Member

glennw commented Mar 28, 2017

@mrobinson You may be interested in this PR I just opened then :) https://github.com/servo/webrender/pull/1015/files#diff-d368cc4d16f067bc1480e7806819a74eR171

@glennw
Copy link
Member

glennw commented Mar 28, 2017

The diff I mentioned adds support for one extra clip, which the box-shadows use. This can trivially be expanded to support an array of clips (all the work for supporting multiple clips is included in this PR).

@mrobinson
Copy link
Member

Here is some more information about the kind of situation we want to support:

<html>
  <div id="1" style="overflow:hidden;">
    <div id="2" style="clip: ...">
      <div id="3" style="position:fixed">
      </div>
    </div>
  </div>
</html>

Frame tree (roughly)

nsCanvasFrame(html) <
  nsHTMLScrollFrame(1) <
    nsBlockFrame(1) <
      nsBlockFrame(2) <
        nsPlaceholderFrame(3) -> nsBlockFrame(3)
      >
    >
  >
>
FixedList <
  nsBlockFrame(3)
>

Note that we have two different types of clipping here, the clip to make overflow:hidden work, and the clip from the css 'clip' property. These work differently. The overflow clip doesn't apply to the out of flow frame, but the css one does.

To handle this in Gecko, we build and store two clips as we descend into the frame tree: GetClipChainForContainingBlockDescendants (for overflow:hidden) and GetClipChainForContentDescendants (for css clip).

We descend into the FixedList before we descend into normal child lists and store the current visible/dirty rect, as well as the current clip for containing block descendants (so without the overflow:hidden clip).

When we descend into the normal child lists and eventually reach the placeholder, the current clip state has both of the clips applied. At this point we restore the state we saved earlier (which removes the overflow:hidden clip, but leaves the css one), and then we build items for the position:fixed block frame.

@mrobinson
Copy link
Member

I think the only thing left here is to support the case where we want to have a clip that is scrolled by not clipped by any parent scroll roots between itself and the reference frame. This is a corner case in Gecko.

@staktrace
Copy link
Contributor

For the record I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1409442 to track this on the bugzilla side.

@staktrace staktrace mentioned this issue Oct 30, 2017
Closed
@mrobinson
Copy link
Member

This should be supported by #2300, though it's unclear if this is the easiest API to use for all the cases that Gecko needs.

mrobinson added a commit to mrobinson/webrender that referenced this issue Jan 15, 2018
@mrobinson
Add support for API-defined ClipChains
000d7a9 
API-defined clip chains allow constructing combinations of clips that
select any combination of clips from the ClipScrollTree without regard
to parents of clips. A ClipChain is defined by an optional ClipChain
parent and a vector of clips to use for that chain. A chain's clips are
combined with its parents clips to provide all the clips for display
items that set their clipping ClipId as the clip chain via
ClipId::ClipChain.

Fixes servo#840.
Fixes servo#1964.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mrobinson mrobinson

Labels
bugzilled
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mrobinson @kvark @staktrace @mstange @glennw