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

Wording about canvas/svg is a bit inaccurate #50

Closed
noamr opened this issue Mar 2, 2020 · 29 comments
Closed

Wording about canvas/svg is a bit inaccurate #50

noamr opened this issue Mar 2, 2020 · 29 comments

Comments

@noamr
Copy link
Contributor

noamr commented Mar 2, 2020

The wording says "non-white canvas or SVG", I think this is misleading. Isn't the meaning here non-empty SVG, and canvas that was painted into?

@npm1
Copy link
Contributor

npm1 commented Mar 2, 2020

I didn't write that part, I believe the intended meaning was to prevent a canvas/SVG that's completely invisible from counting as FCP. However, in practice it can be very expensive to do such checks, and in fact I believe we don't do such checks in Chromium. We could change to non-empty, but are empty SVG/canvas a thing? Sorry, not very familiar with those objects.

@noamr
Copy link
Contributor Author

noamr commented Mar 2, 2020

It's not about invisibility, but rather about canvas that had a context that was painted into. I'm not sure what the terminology for that is in spec-land.

@noamr
Copy link
Contributor Author

noamr commented Mar 2, 2020

Maybe the term should be "a canvas with an initialized output bitmap"? https://html.spec.whatwg.org/#output-bitmap

@npm1
Copy link
Contributor

npm1 commented Mar 2, 2020

It says there that the object is initialized upon creation of the canvas, so I'm not sure what you mean.

@noamr
Copy link
Contributor Author

noamr commented Mar 2, 2020

It says "A CanvasRenderingContext2D object has an output bitmap that is initialized when the object is created.". It means that it's created when a context is attached to the canvas. Maybe the wording should be "a canvas that has a context attached to it" or "a canvas that has a context attached to it, where actions that were performed on the context have been rendered"... not sure of exact way to put this

@npm1
Copy link
Contributor

npm1 commented Mar 2, 2020

Right but a canvas does not need to have a 2D context, right? At least that's what I understand from this table: https://html.spec.whatwg.org/#the-canvas-element:2d-context-creation-algorithm

@noamr
Copy link
Contributor Author

noamr commented Mar 2, 2020

Right. a canvas without a painting context (2d/webgl etc) is definitely not contenful.

@rniwa
Copy link

rniwa commented Mar 3, 2020

I think we want something like the bitmap of the canvas ever been modified from the initial transparent black state. Note that canvas' context mode can be "none", "placeholder", "bitmaprenderer", "webgl", or "webgl2", and each context mode requires its own definition of being in the initial state.

See https://html.spec.whatwg.org/multipage/canvas.html#the-canvas-element

@noamr
Copy link
Contributor Author

noamr commented Mar 3, 2020

I like it. How about "reporting the time when the browser first rendered any text, image (including background images), canvas that has been altered from its initial state, or SVG

@rniwa
Copy link

rniwa commented Mar 3, 2020

That's the essence of what we want to say but that sentence is still too vague / imprecise. We may need to refactor the HTML spec to define a concept here.

@annevk @dbaron: any opinions / feedback from Mozilla about this?

@annevk
Copy link
Member

annevk commented Mar 3, 2020

Would any action taint it, including drawing a fully transparent black image? If so, seems reasonable, but would require a lot of changes to properly define from first principles.

@npm1
Copy link
Contributor

npm1 commented Mar 3, 2020

It seems hard to keep track of the state of the canvas everywhere. What about only discarding canvas whose context mode is "none"?

@noamr
Copy link
Contributor Author

noamr commented Mar 3, 2020

It seems hard to keep track of the state of the canvas everywhere. What about only discarding canvas whose context mode is "none"?

I like the idea

@rniwa
Copy link

rniwa commented Mar 3, 2020

What about only discarding canvas whose context mode is "none"?

What does this mean?

If you’re saying that we should special case the context mode of “none” then I don’t think we should because there is no rendering difference to the end user. The end user doesn’t care if what they see is a fallback content, 2D canvas, or WebGL drawing.

@noamr
Copy link
Contributor Author

noamr commented Mar 3, 2020

What about only discarding canvas whose context mode is "none"?

What does this mean?

If you’re saying that we should special case the context mode of “none” then I don’t think we should because there is no rendering difference to the end user. The end user doesn’t care if what they see is a fallback content, 2D canvas, or WebGL drawing.

It means that no context was created for the canvas yet, e.g. no one called canvas.getContext('2d' / 'webgl'), therefore it's clear that nothing was rendered into it. It leaves out the edge case where, for example, someone created a context and painted nothing, or created a context and painted only transparent pixels.

@annevk
Copy link
Member

annevk commented Mar 3, 2020

How often does that happen though to add all the additional branching required? Taking a step back whenever I see creation of a canvas element it's almost always followed by a getContext() call and some operations on the return value of that.

@noamr
Copy link
Contributor Author

noamr commented Mar 3, 2020

How often does that happen though to add all the additional branching required? Taking a step back whenever I see creation of a canvas element it's almost always followed by a getContext() call and some operations on the return value of that.

The scenario where there is a canvas element in the DOM but the JavaScript that paints to it didn’t run yet (e.g. it’s inside a requestAnimationFrame callback or in a slow loading library) is very plausible

@rniwa
Copy link

rniwa commented Mar 4, 2020

How often does that happen though to add all the additional branching required? Taking a step back whenever I see creation of a canvas element it's almost always followed by a getContext() call and some operations on the return value of that.

The scenario where there is a canvas element in the DOM but the JavaScript that paints to it didn’t run yet (e.g. it’s inside a requestAnimationFrame callback or in a slow loading library) is very plausible

Do we have any data suggesting that websites don't create the context right away in those scenarios? I've definitely seen code where people just create the context right away after setting width, height etc... as a part of the initial setup.

@npm1
Copy link
Contributor

npm1 commented Mar 4, 2020

I think what @noamr is saying is that you could have something like <canvas id="myCanvas" width="200" height="100"></canvas> and have JS to fill content inside it.

@annevk
Copy link
Member

annevk commented Mar 4, 2020

Yeah, the question is how common that is. If it's not common I'm not convinced it's worth adding complexity for (and would instead be something a lint tool might recommend against).

@noamr
Copy link
Contributor Author

noamr commented Mar 4, 2020

Yeah, the question is how common that is. If it's not common I'm not convinced it's worth adding complexity for (and would instead be something a lint tool might recommend against).

It's very common and possible the way most canvas sites are built!
See for example all the samples at https://webglsamples.org/. The canvas is part of the DOM with a fixed size, and getContext('webgl') is only called when jquery and tdl (Google's 3rd part library) are loaded.

@noamr
Copy link
Contributor Author

noamr commented Mar 4, 2020

Also see my comment in a different thread: #58 (comment). Suggesting to have a strict spec of what's considered contentful, but allow implementors to have a "fast path".

@noamr
Copy link
Contributor Author

noamr commented Mar 5, 2020

So, I suggest changing the wording from "NOTE: This paint must include text, image (including background images), non-white canvas or SVG." to "NOTE: This paint must include text, image (including background images), a canvas that does not have a rendering context bound to it, or SVG."

@noamr
Copy link
Contributor Author

noamr commented Mar 5, 2020

it fits with the wording in the HTML spec: "A canvas element can have a rendering context bound to it. Initially, it does not have a bound rendering context."

@rniwa
Copy link

rniwa commented Mar 5, 2020

So, I suggest changing the wording from "NOTE: This paint must include text, image (including background images), non-white canvas or SVG." to "NOTE: This paint must include text, image (including background images), a canvas that does not have a rendering context bound to it, or SVG."

This can't really be a note. It needs to be a normative text. Also, please refer to the concept of context mode instead of using colloquial terminology.

Every term used in each spec needs to be precisely defined.

@noamr
Copy link
Contributor Author

noamr commented Mar 6, 2020

So, I suggest changing the wording from "NOTE: This paint must include text, image (including background images), non-white canvas or SVG." to "NOTE: This paint must include text, image (including background images), a canvas that does not have a rendering context bound to it, or SVG."
Also, please refer to the concept of context mode instead of using colloquial terminology.

Every term used in each spec needs to be precisely defined.

It refers to the lines in the html spec: “ A canvas element can have a rendering context bound to it. Initially, it does not have a bound rendering context.”
Of course it needs to be normative etc - bit it does refer to existing terms.

@rniwa
Copy link

rniwa commented Mar 6, 2020

So, I suggest changing the wording from "NOTE: This paint must include text, image (including background images), non-white canvas or SVG." to "NOTE: This paint must include text, image (including background images), a canvas that does not have a rendering context bound to it, or SVG."
Also, please refer to the concept of context mode instead of using colloquial terminology.

Every term used in each spec needs to be precisely defined.

It refers to the lines in the html spec: “ A canvas element can have a rendering context bound to it. Initially, it does not have a bound rendering context.”
Of course it needs to be normative etc - bit it does refer to existing terms.

Since there is no definition for the term "rendering context", it's dubious at best to refer to it in another specification. Filed whatwg/html#5338 to track that issue. We can resolve that issue in the HTML specification if we wanted to use it. However, I'd wager it's better / easier to use an equivalent term that's already formally defined.

@noamr
Copy link
Contributor Author

noamr commented Mar 11, 2020

I believe that this is addressed by #66

@noamr
Copy link
Contributor Author

noamr commented Mar 20, 2020

Closing, https://w3c.github.io/paint-timing/ now has precise definition of contentulness wrt canvas and SVG.

@noamr noamr closed this as completed Mar 20, 2020
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

No branches or pull requests

4 participants