Skip to content

[css-paint-api] scope of paint-valid flag being box (not box+use/size) seems wrong #326

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

Closed
dbaron opened this issue Oct 26, 2016 · 3 comments
Assignees

Comments

@dbaron
Copy link
Member

dbaron commented Oct 26, 2016

In the section on Paint Invalidation and the section on Drawing an Image, the Painting API specification describes the concept of a paint-valid vs. paint-invalid flag.

It seems to describe this flag as being per-box, whereas I think it should either be:

  • both per-box and per-use, or
  • both per-box and per-concrete object size

In particular, if a style sheet has something like:

  background-image: paint(foo), paint(foo);
  background-size: 50px 50px, 100px 100px;
  background-position: top left, top right;

then the two images need to be painted separately, since they're different sizes, and the paint function would expect to be called separately for each size. But if they were the same size, the implementation could presumably call the paint function only once (which is why I suggest per-concrete object size rather than per-use). Though making it per-concrete object size requires changing a bit of the invalidation logic in the section on Paint Invalidation.

There are similar problems for two uses of the same paint() function in different properties on the same element.

(I got here via w3ctag/design-reviews#140 .)

@dbaron
Copy link
Member Author

dbaron commented Oct 26, 2016

Though I'd also note that maybe refactoring the invalidation logic around sizes is a good idea anyway, given that the above case does not require any invalidation if the element's size changes, since the concrete object size does not change, since the background-size is fixed and does not depend on the size of the element.

(I'm going to be lazy and leave that as part of this issue rather than filing a separate one... at least for now.)

bfgeek added a commit that referenced this issue Apr 16, 2017
…gents to cache.

Fixes #326.

I realized that explicitly placing an invalidation based on size here
was wrong, e.g. if a box changes size it has to run the "object size
negotiation" algorithm, which makes invalidating the paint() function
off size redundant.

Additionally allows user-agents to cache results from previous
invocations, and re-use. This allows UAs to re-use images between
instances if they have the same input style, concrete object size, and
input arguments.
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Issue 326.

The full IRC log of that discussion
<Rossen> Topic: Issue 326
<fantasai> iank_: not invalid if the box size changed, but not necessarily correct
<dbaron> from https://drafts.css-houdini.org/css-paint-api/#paint-invalidation iank wants to remove: When the size (as determined by layout) of a box changes, each <paint()> function’s paint valid flag should be set to paint-invalid.
<dbaron> Github topic: https://github.com/w3c/css-houdini-drafts/issues/326
<fantasai> iank_: This can just work thorugh other CSS machinery, so don't have to eplicitly invalidate
<fantasai> iank_: It will just call this function when it needs to
<fantasai> iank_: So I think just by removing this line here, would resolve this issue
<fantasai> Rossen: Whe we added this, what was the main reason. Whasi it invalidation?
<fantasai> iank_: This was in the spec from ay 1.
<fantasai> Rossen: From what I recall, this was there to capture the cases where you have to repaint?
<fantasai> iank_: Paint thing is there if it's in the viewport ... you have to paint it
<fantasai> iank_: If box size changes anyway, then UA will repaint it anyway
<fantasai> dbaron: The paint() function cnan be used mutliple times per element
<fantasai> dbaron: If box size changes, maybe will need to repaint, maybe not
<fantasai> dbaron: What you want to track is whether you're valid for this element at this size
<dbaron> (I was just summarizing the initial comment of https://github.com/w3c/css-houdini-drafts/issues/326 )
<fantasai> iank_: But invalidates always whenver the box changes, so I think we can remove that and rely on CSS machinery to cal you if the concrete size changes
<fantasai> smfr: Does the output of the paint have an intrinsic size, as CSS understands intrinsic size?
<fantasai> iank_: I think it doesn't have an intrinsic size
<fantasai> ChrisL: Why should this wait for later version fothe spec? Seems useful
<fantasai> iank_: Just to keep inital version ismpler
<fantasai> iank_: Does it make sense, then , to rely on the engien to call it?
<fantasai> dbaron: You should have something that makes the paint valid flag invalid when the actual thing you need to paint has changed size
<fantasai> iank_: So I'm saying we .. invalidation, it's at a higher level in CSS
<fantasai> fantasai: Maybe you just need to chnage the conditional on that line, to refer to changes in concrete size of the painted image, rather than changes in the box
<dbaron> https://drafts.css-houdini.org/css-paint-api/#drawing-an-image
<fantasai> dbaron: Might be worht taking this offline

@dbaron
Copy link
Member Author

dbaron commented Apr 18, 2017

After looking more closely at #390, I think the changes there are good, modulo one wording nit.

bfgeek added a commit that referenced this issue Apr 19, 2017
#390)

* [css-paint-api] Removes invalidation based on size, and allows user-agents to cache.

Fixes #326.

I realized that explicitly placing an invalidation based on size here
was wrong, e.g. if a box changes size it has to run the "object size
negotiation" algorithm, which makes invalidating the paint() function
off size redundant.

Additionally allows user-agents to cache results from previous
invocations, and re-use. This allows UAs to re-use images between
instances if they have the same input style, concrete object size, and
input arguments.
@tabatkins tabatkins removed the ready label Apr 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants