-
Notifications
You must be signed in to change notification settings - Fork 143
[css-paint-api] Calls to paint function should provide the snapped concrete size. #508
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
Labels
Comments
As en example of each of these strategies, see this code example and blown up image below: |
The Working Group just discussed
The full IRC log of that discussion<iank_> Topic: Concrete object size should be the snapped pixel size?<iank_> github: https://github.com//issues/508 <TabAtkins> iank_: In th eissue, flackr has a detailed descriptionof what we want to do. <TabAtkins> iank_: Involves pixel snapping of iamges <TabAtkins> iank_: Happens a lot <TabAtkins> iank_: Proposing #3 on this list, to adjust concrete object size to snapped pixel size / device pixel ratio <TabAtkins> iank_: So if people just naively draw into the size get the expected beahvior. <TabAtkins> iank_: If people want to draw a perfect pixel-aligned line, they can multiple by pixel ratio <TabAtkins> TabAtkins: So they might get like a 10.5px value for th eobject size? <TabAtkins> iank_: Yes <TabAtkins> smfr: If you ahvea transformed ancestor this doesn't affect that, right? <TabAtkins> iank_: Yeah <TabAtkins> smfr: Like we do today with paintaing backgrounds? <TabAtkins> iank_: Yes. Matches what we do internally. <TabAtkins> dbaron: I think our transform behavior doesn't match this. We try in some cases to rasterize at higher resolutions. <TabAtkins> dbaron: We might have changed that. <TabAtkins> mwoodrow: We still do that. <TabAtkins> smfr: We do that too <TabAtkins> smfr: If you do scale(2) we'll rasterize at higher resolution <TabAtkins> smfr: But I thin kthe snapping we do ignores transforms <TabAtkins> TabAtkins: In the general case you have to; it might not be axis-aligned <TabAtkins> dbaron: I think if the transform is only a translate/scale, we try to honor that with higher fidelity. <TabAtkins> TabAtkins: Do we want to try and make the spec generic to platform behavior? <TabAtkins> dbaron: Maybe - we should probably converge at some point. <TabAtkins> iank_: Nice thing is that this approach lets naive behavior work right, regardless <TabAtkins> TabAtkins: I can help with the spec text if we want to allow platform divergence here. Testing is harder tho. <TabAtkins> TabAtkins: But if we want to spec a single behavior that's a different thing. <TabAtkins> iank_: Yeah, that requires more convergence. <TabAtkins> TabAtkins: So resolution is, *whatever* the browser's snapping bheavior is, it must apply in the way described in #3? <TabAtkins> Rossen: And #4 means no snapping? <TabAtkins> Rossen: Because backing stor epixel ratio is what gets you back to physical pixels, so conversion happens in user code, nobody has to muck around with adjust the size <TabAtkins> Rossen: So whatever Gecko does... <TabAtkins> iank_: If we give people physical pixels, that has a bunch more interop issues. <TabAtkins> iank_: If we end up giving you a much larger backing store and Gecko doesn't, and you naively assume its the CSS size, it gives us interop pain. <TabAtkins> iank_: It might be a better option later on to actually request physical pixels, but for now we have the naive possibly-fractional CSS px <TabAtkins> TabAtkins: What does the backing store look like if we ahve a 2x pixel ratio? <TabAtkins> iank_: First, it might be different from the device pixel ratio, due to scales <TabAtkins> iank_: You can't see the underlying pixel ratio of the canvas in the worklet. <TabAtkins> TabAtkins: Ah, that satisfies my concern. <TabAtkins> iank_: And later we can add a flag to ask for th ephysical size if you really want to draw hairlines or something. <TabAtkins> smfr: A general comment - you should get the same rendered result whether the UA stores a display list or a bitmap <TabAtkins> smfr: Question - does the context start with a scale for the dpr? <TabAtkins> iank_: That's how we do it internally. The backing size is the snapped pixel size scaled by the dpr. <TabAtkins> iank_: So people can just use naive CSS px, which they like, and they still get sharp lines. <TabAtkins> iank_: Can add a note suggesting that for impls. <TabAtkins> smfr: Do you give the author enough info to reproduce the browser snapping behavior? <TabAtkins> smfr: So when the brwoser pixel snaps, for us, we consult the rectangle origin - we'll snap both origin and width using some complex rules. <TabAtkins> smfr: If the person using the paint worklet wants to snap in same way, that would be hard in a cross-browser way <TabAtkins> iank_: I think we convinced ourselves internally that we didn't need to worry about th eorigin. <flackr> Because the origin is snapped and you know the number of snapped pixels you can match browser snapping <TabAtkins> smfr: So say you want to paint a line halfway in the background, matching the browser behavior for a 50% width box with a border . <TabAtkins> smfr: Matching the browser snapping behavior there is non-obvious. <TabAtkins> smfr: Not saying we need to solve immediately, and probably would be hard to do and expose unsavory browser internals, but it should be an issue. <flackr> In the jsbin demo code I show how you'd convert to native size <TabAtkins> iank_: Trying to read a big number-heavy thread right now for this. <TabAtkins> smfr: could imagine a .snappedPoint/Rect() function, that you feed a point/rect and it returns a snapped one. <TabAtkins> iank_: [reads out from teh thread, where a proposal matches what smfr just said] <TabAtkins> iank_: So future level, but we should open an issue for that. <TabAtkins> TabAtkins: So back to original proposal. <TabAtkins> smfr: I like #3 <TabAtkins> dbaron: I don't think I understand all the details, but #3 seems to be okay. <TabAtkins> TabAtkins: I don't either, but if you alter understand the details and disagree, we can revisit. <TabAtkins> RESOLVED: Accept proposal #3 for pixel-snapping the backing store <TabAtkins> RESOLVED: Add an issue for exposing the browser's snapping behavior via an API in a future level. |
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this issue
Nov 14, 2017
Right now when we set the PaintSize in CSSPaintDefinition::Paint(), the size is the concrete object size. There has been some discussion around here: w3c/css-houdini-drafts#508 and the solution is to pass the snapped concrete object size divided by device scale factor. The ongoing pull request to change the spec is here: w3c/css-houdini-drafts#518 This CL corrects the PaintSize to reflect the spec changes. It also adds a layout test to ensure the correctness. Bug: 774810 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I42b57db6ac0d40be2b408cb733a659f5ea14dfa0
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this issue
Nov 14, 2017
Right now when we set the PaintSize in CSSPaintDefinition::Paint(), the size is the concrete object size. There has been some discussion around here: w3c/css-houdini-drafts#508 and the solution is to pass the snapped concrete object size divided by device scale factor. The ongoing pull request to change the spec is here: w3c/css-houdini-drafts#518 This CL corrects the PaintSize to reflect the spec changes. It also adds a layout test to ensure the correctness. Bug: 774810 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I42b57db6ac0d40be2b408cb733a659f5ea14dfa0
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this issue
Nov 15, 2017
Right now when we set the PaintSize in CSSPaintDefinition::Paint(), the size is the concrete object size. There has been some discussion around here: w3c/css-houdini-drafts#508 and the solution is to pass the snapped concrete object size divided by device scale factor. The ongoing pull request to change the spec is here: w3c/css-houdini-drafts#518 This CL corrects the PaintSize to reflect the spec changes. It also adds a layout test to ensure the correctness. Bug: 774810 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I42b57db6ac0d40be2b408cb733a659f5ea14dfa0
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this issue
Nov 15, 2017
Right now when we set the PaintSize in CSSPaintDefinition::Paint(), the size is the concrete object size. There has been some discussion around here: w3c/css-houdini-drafts#508 and the solution is to pass the snapped concrete object size divided by device scale factor. The ongoing pull request to change the spec is here: w3c/css-houdini-drafts#518 This CL corrects the PaintSize to reflect the spec changes. It also adds a layout test to ensure the correctness. Bug: 774810 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I42b57db6ac0d40be2b408cb733a659f5ea14dfa0 Reviewed-on: https://chromium-review.googlesource.com/733862 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#516872}
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this issue
Nov 15, 2017
Right now when we set the PaintSize in CSSPaintDefinition::Paint(), the size is the concrete object size. There has been some discussion around here: w3c/css-houdini-drafts#508 and the solution is to pass the snapped concrete object size divided by device scale factor. The ongoing pull request to change the spec is here: w3c/css-houdini-drafts#518 This CL corrects the PaintSize to reflect the spec changes. It also adds a layout test to ensure the correctness. Bug: 774810 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I42b57db6ac0d40be2b408cb733a659f5ea14dfa0 Reviewed-on: https://chromium-review.googlesource.com/733862 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#516872}
MXEBot
pushed a commit
to mirror/chromium
that referenced
this issue
Nov 16, 2017
Right now when we set the PaintSize in CSSPaintDefinition::Paint(), the size is the concrete object size. There has been some discussion around here: w3c/css-houdini-drafts#508 and the solution is to pass the snapped concrete object size divided by device scale factor. The ongoing pull request to change the spec is here: w3c/css-houdini-drafts#518 This CL corrects the PaintSize to reflect the spec changes. It also adds a layout test to ensure the correctness. Bug: 774810 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I42b57db6ac0d40be2b408cb733a659f5ea14dfa0 Reviewed-on: https://chromium-review.googlesource.com/733862 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#516872}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Section 8: Drawing an image specifies that the Concrete Object Size is used when invoking the paint callback, however in the case of subpixel sizes the actual paint size depends on the browser's subpixel snapping.
For example, given a device with a scale factor of 1.25 if drawing a series of boxes of size 9px, the physical screen size is 11.25px. This means that one in every 4 boxes will be drawn a physical pixel taller than the rest.
There are 4 options I can think of although I believe we should avoid the first two as neither allows developers to do the right thing:
My preference is for option 3 given it is a small change, and it produces crisp edges for developers naively painting to their size, and do the right thing for developers taking into account the devicePixelRatio as well.
The text was updated successfully, but these errors were encountered: