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

Fix computed value computation #96

Merged
merged 2 commits into from
Feb 24, 2021
Merged

Fix computed value computation #96

merged 2 commits into from
Feb 24, 2021

Conversation

npm1
Copy link
Collaborator

@npm1 npm1 commented Feb 24, 2021

When determining whether the node can be considered unstable, we need to consider not just the node but also its ancestors, since computed value does not account for inheritance. Fixes #61


Preview | Diff

Verified

This commit was signed with the committer’s verified signature.
clo4 Robert Clover
When determining whether the node can be considered unstable, we need to consider not just the node but also its ancestors, since computed value does not account for inheritance. Fixes #61
@npm1 npm1 requested a review from skobes-chromium February 24, 2021 15:36
@skobes-chromium
Copy link
Collaborator

It looks like visibility is inherited, and can be overridden. Should we do this for opacity only?

https://output.jsbin.com/cevoned/quiet

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@npm1
Copy link
Collaborator Author

npm1 commented Feb 24, 2021

Ahh interesting, done.

@npm1 npm1 merged commit c204348 into master Feb 24, 2021
@npm1 npm1 deleted the fix-computed-value branch February 24, 2021 18:15
@dholbert
Copy link
Contributor

The new spec text looks good -- but let me correct the motivation/reasoning a bit. The commit message said:

[..] since computed value does not account for inheritance.

In fact, the computed value does account for inheritance; and the scenario that we're addressing here doesn't have to do with opacity inheriting or not-inheriting.

The reason that we have to check ancestors (for opacity) is that opacity simply has its effect by changing the painting of the element's whole subtree (by e.g. creating a GPU layer for the element, and then configuring the opacity of that layer on the GPU). So this is why opacity:0 on any ancestor-box will trivially mean that we can be sure that our own box is guaranteed not to paint, regardless of our own box's opacity value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

layout instability spec needs to clarify whether (and which) unpainted nodes are considered or not
3 participants