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.

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

Document and clarify `push_gradient` behavior #2222

Closed
pyfisch opened this issue Dec 13, 2017 · 1 comment
Closed

Document and clarify `push_gradient` behavior #2222

pyfisch opened this issue Dec 13, 2017 · 1 comment

Comments

@pyfisch
Copy link
Contributor

@pyfisch pyfisch commented Dec 13, 2017

cc @eqrion

How exactly should DisplayListBuilder::push_gradient behave?

The signature is:

pub fn push_gradient(
    &mut self, 
    info: &LayoutPrimitiveInfo, 
    gradient: Gradient, 
    tile_size: LayoutSize, 
    tile_spacing: LayoutSize
)

My understanding is is the following:

  • param info contains information common to most "elements". Most notably a rectangle describing where on screen the "element" is located. This rectangle is filled with the gradient in this case. What does the local_clip attribute do?
  • param gradient, well contains information about the actual gradient.
  • param tile_size describes the area on which the gradient should be drawn. If its size differs from the rectangle in infomultiple "tiles" are drawn next to each other.
  • param tile_spacing spaces out the tiles specified in tile_size. Which CSS features require to set this value to something else then zero?

Firefox:
Firefox
Servo (via webrender):
Servo
(To see this servo rendering you need servo/servo#19554)
Source:
https://gist.github.com/pyfisch/339a41b88e3b794f88be1d9c818ad09e

The Servo rendering looks like the tile is only repeated to the right and bottom but not to the other sides. Is this intentional?
CSS supports repeating only in axis (repeat-x, repeat-y) or no repeating at all. How should this be expressed?

  1. Restrict the rectangle passed inside info to the area where the gradient should actually be displayed.
  2. Use std::f64::MAX or similar values in tile_spacing to hide other instances of the tile?

What is preferable performance and semantics wise?

@eqrion
Copy link
Contributor

@eqrion eqrion commented Dec 14, 2017

@pyfisch Hi! First a disclaimer that I have not worked on webrender directly for a couple of months so my knowledge may be out of date.

If you don't mind reading some C++ code, here is where Gecko actually creates webrender gradient display items.

Your understanding of the parameters seems correct. I believe that tile_size is needed for background-repeat: space

My best recollection of how Gecko handles tile repeating and not tile repeating, is that Gecko will calculate the minimum X,Y position of the first visible tile and place the display item there. That should ensure fix the case where the gradient doesn't appear to repeat up and to the right. Then we will size it for how many tiles are to be visible (1 or many). I think that is option 1 of what you describe.

Let me know if anything is unclear.

pyfisch added a commit to pyfisch/webrender that referenced this issue Dec 26, 2017
bors-servo added a commit that referenced this issue Jan 2, 2018
Document push_(radial_)gradient

Closes #2222.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2248)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jan 2, 2018
Document push_(radial_)gradient

Closes #2222.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2248)
<!-- Reviewable:end -->
GuanWen-Chen added a commit to GuanWen-Chen/webrender that referenced this issue Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.