search

w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.52k stars 673 forks source link

[resize-observer] device-pixel-border-box size #3554

Closed gregwhitworth closed 5 years ago

gregwhitworth commented 5 years ago

device-pixel-border-box size

device-pixel-border-box size is Element's border-box size in device pixels. It is always an integer, as there are no fractional device pixels. It can currently be approximated by Math.round(borderBoxSize * window.devicePixelRatio), but it cannot be computed exactly, because native code uses a different rounding algorithm.

Use case

This unusual size request comes from Chrome's WebGL canvas team. It solves the long standing WebGL developers problem: "How to create HiDPI canvas without moire pattern?".

The existing "best practice" for creating a HiDPI canvas is to set size of canvas context to a multiple of canvas's css width/height. Example:

let cssWidth = 300, cssHeight = 200;
let c = document.querySelector('canvas');
c.style.width = cssWidth + "px";
c.style.height = cssHeight + "px";
let ctx = c.getContext("webgl2");
ctx.width = cssWidth * window.devicePixelRatio;
ctx.height = cssHeight * window.devicePixelRatio;

The webgl context will be HiDPI, one one canvas pixel should correspond to one device pixel. But, because ctx.width is pixel snapped, ctx.width can differ from "true" device pixel width. This difference can cause visible moire patterns when rendered.

Because of this, WebGL team believes that web platform needs to provide an API for true device pixel width.

Discussion

This size has several interesting differences from others reported by ResizeObserver:

Q: Does this size belong to ResizeObserver, or should we create a diferent DOM API for it?

I can't think of a clean API that would provide same functionality. Web developers must observe this size, and respond to its changes. ResizeObserver is the only size-observing API. Observing border-box size, and providing "devicePixelSize()" method will not work, because devicePixelSize could change without border-box changing.

Q: Should we observe device-pixel-size on all elements, or just canvas?

Observing device-pixel-size comes with performance cost, because size must be checked when Element's position changes. For all other sizes, we do not need to check size when position changes. Weak preference: Only allow device-pixel-size observation for canvas.

Q: Should we report device-pixel-size on all elements, or just canvas?

Weak preference: make it canvas-only, because other elements cannot observe this size.

Originally posted by @atotic in https://github.com/w3c/csswg-drafts/issues/3326#issuecomment-440041374

chrishtr commented 5 years ago

Here is a demo page that clearly shows the issue:

https://output.jsbin.com/gemipizazi/1

If you load this in Chrome Canary with the --enable-use-zoom-for-dsf command-line flag (*), and resize the browser window vertically, there will be a number of visual artifacts in the rendering, that appear or disappear according to the different pixel snapping that happens with different viewport heights.

If you load the same demo in Safari Technical Preview with the experimental ResizeObserver turned on, and resize the browser vertically, you'll see the same kind of artifacts (but different ones, because the pixel snapping algorithm depends on the browser).

If you apply a hacked-up prototype of the device pixel border box observation feature to Chromium via patching this CL then the rendering artifacts are gone.

I tested these on a Retina MacBook Pro.

(*) This flag turns on pixel snapping at device pixel granularity, rather than CSS pixel granularity. This flag has already been shipped in Chrome on all platforms except for Mac, and will ship on Mac soon. It's also a necessary feature for high quality output, but orthogonal & complementary to the issue being discussed here.

chrishtr commented 5 years ago

I have added this issue to the agenda for next week's teleconference.

atotic commented 5 years ago

Another demo revision: https://jsbin.com/soqexor

css-meeting-bot commented 5 years ago

The CSS Working Group just discussed device-pixel-border-box size, and agreed to the following:

The full IRC log of that discussion <dael> Topic: device-pixel-border-box size
<dael> github: https://github.com/w3c/csswg-drafts/issues/3554#issuecomment-538465771
<dael> astearns: Discussed device-pixel-border-box size at TPAC
<dael> astearns: I recall we still weren't sure how change will effect Safari. Example have now been provided. Do we have enough information on impact to Safari?
<dael> smfr: Saw a couple of examples. I think Chris said he ran the tests on Macbook pro retina with the fixes and it made it look better. Can't say without doing work in webkit, but I think sufficient proof this is useful. On non-apple it's well known it's necessary
<dael> astearns: Any other new information?
<dael> ??: Nothing new except demos and version of chrome that fixes
<astearns> s/??/chrishtr/
<dael> smfr: I'm surprised you could get good results. Most macbooks have default scaling. Surprised you got good results on a machine with that behavior
<dael> chrishtr: It's not the newest
<drousso> presnet+
<dael> [audio problems
<dael> chrishtr: Tested on not absolute newest, but a macbook pro retina. I think it does have value and OS scaling if it's outside of control of application those situations can't be fixed
<dael> smfr: What I'm interested in understanding is if on hardware with builtin scaling suc htat you never get pixel perfect is there improvement in device-pixel-border-box ? Is there value in making it optional and allow UA to not provide if it thinks it can provide better. Or do we make it required and force UA to do this when it's not going to get better
<dael> myles: Easy to test if you change resolution of OS in your macbook pro
<dael> astearns: Sounds to me like we don't have an objection to adding device-pixel-border-box size to the API, but may want another issue about behavior of API possibly making it optional?
<dael> chrishtr: Would app fallback be multiply css pixel width and height by device-pixel ratio if UA doesn't supply?
<dael> smfr: Impl will alreayd have to multiply by device-pixel-ratio. No, does multiply. Okay.
<dael> chrishtr: That was the point, rounding or flooring can't get consistent
<dael> smfr: Okay, then makes optional thing annoying
<dael> chrishtr: I don't know if built in scaling is higher quality but we got really good results on macbook pros even with non-1 to 1 scaling.
<dael> smfr: Okay
<dael> s/ chrishtr / ken
<dael> smfr: Not objecting to adding. Question if needs to be rectangle or just a size since left and top don't mean anything
<dael> chrishtr: Use case for canvas sizing top left not nec
<dael> smfr: We don't have a dom size object
<dael> chrishtr: Yep. More consistent to have a rect, rect does have meaning. Not used in canvas case
<dael> ken: Position of rect a little confusing if you consider overflow area
<dael> chrishtr: Does mean where it is on device window though
<astearns> s/ken/myles/
<dael> chrishtr: It's used within the native texture if there's not special scaling smfr referred to. If texture is scrolled it's still...it doesn't take into account transforms and scrolls
<dael> myles: Values off the top of the screen?
<dael> chrishtr: I think would be fine to have size only for this reason
<dael> chrishtr: top left doesn't seem to mean much, want to know canvas size
<dael> astearns: Array of 2 values or are you minting a new size objection?
<dael> chrishtr: I don't know. Maybe new size object?
<astearns> s/objection/object/
<dael> smfr: If it always integral?
<dael> chrishtr: Yeah
<dael> smfr: Maybe you jsut have two long on the entry
<dael> chrishtr: THat are just width and height
<dael> chrishtr: As relates to desire to add the eq method to getBoundingClientRect it would change to get device pixel width and height
<dael> smfr: If we agree to resizeObserver do we still need?
<dael> chrishtr: Don't think so, but ?? from Mozilla thought case was strong so I was okay adding
<dael> smfr: Which case was it?
<astearns> s/??/Jeff Gilbert/
<dael> chrishtr: You have a full screen where you don't want resizeObserver and you want a slight jump on resize frames
<dael> ken: Jeff also had concerns that it fires late and application would fall behind a frame which is legit concern
<dael> myles: You would need to executecode every frame with this. resizeObserver means code only called when need to be.
<fantasai> s/executecode/execute heavy calculations/
<dael> ken: Agree, but in experience from a dev on FF stack we felt it was important to aleviate concern. And FF intends to not recompute if layout isn't dirty
<tantek> regrets+
<dael> smfr: I don't want every getBoundingClientRect to be more expensive
<dael> chrishtr: Adding a new thing
<dael> smfr: Okay, okay. Should discuss sep.
<dael> chrishtr: Okay with me. Is Jeff Gilbert on? Want ot make sure he's okay to separate.
<dael> chrishtr: If he's not, maybe tentatively do that.
<dael> ken: I think Jeff would object. Not to represent his opinion, but I think he would.
<dael> chrishtr: Great to resolve device-pixel-border-box size makes sense and then follow-up on the other one
<dael> chrishtr: To make progress.
<dael> ken: Want progress too
<dael> astearns: smfr you would rather continue new API discussion or resolve to add it and continue discussion?
<dael> smfr: Prefer to fork into separate issue
<dael> fantasai: Question, says doing device-pixel-border-box size. What if person wants size of padding or content box?
<dael> chrishtr: Those other boxes have use cases and tracked via other issues on the spec
<dael> florian: But not at device pixel level the others
<dael> chrishtr: No. Only use case for device pixels is canvas
<dael> fantasai: But why wouldn't people using canvas want to paint inside the border?
<AmeliaBR> Wouldn't it be the content-box that is important for the canvas? Since that's what they're using in their code?
<dael> chrishtr: Canvas has a certain size and border is outside of that. Browser does that for you.
<dael> fantasai: boder box size includes border so if it's not included it's not a border box
<dael> chrishtr: It's the device pixel box size in case of canvas. It's actual size of canvas
<dael> fantasai: Then it's the content box and we should call it that and not border box
<dael> chrishtr: Right
<Zakim> fantasai, you wanted to ask about padding/content boxes
<dael> ken: Sounds good
<dael> astearns: Other discussion?
<dael> fantasai: Summary of proposal?
<dael> astearns: Add an API to get canvas height and width to resizeObserver
<dael> chrishtr: Actual device width and height of the canvas. When changes resizeObserver fires.
<dael> fantasai: I want to be clear. Adding and API which is only available on canavas element. Reflects pixel size of content box of canvas element
<dael> fantasai: And it has a name that is not device-pixel-border-box
<dael> chrishtr: Yes
<dael> smfr: Available for any element you resizeObserve not jsut canvas
<dael> fantasai: Have had various discussion on only canvas or all and want to be clear
<dael> chrishtr: Only use case I know is canvas. Could do it for all
<dael> smfr: I wonder if this should be something the user of the API request.
<dael> chrishtr: For any API you ask for what you want and you're saying only available on canvas?
<dael> smfr: ONly want browser to do computation if author asks for data
<dael> chrishtr: For sure. Part of draft spec to observe multiple types of boxes. You say what to observe. I agree browser should only do this if needed
<dael> astearns: Further question of if someone requests this on not canvas what happens
<dael> chrishtr: Allow or not allow is both okay. In terms of APIs implicity maybe better on all
<dael> smfr: Can imagine on something like video. Better on all
<dael> chrishtr: No impl difficulty from allowing it
<dael> fantasai: Add API to get content box height and width in device pixel sizes to resize observer. ONly return when requested and applies to all.
<dael> chrishtr: Can bikeshed name
<dael> florian: Not obj, but want to be cautios on all elements. pixels vs device pixels people can get confused about and I have mild concern making this too easily available people will try and use it.
<dael> astearns: A lot of things discussing now should be separate issues once we have draft text in place.
<dael> astearns: objections to adding this?
<dael> RESOLVED: Add API to get content box height and width in device pixel sizes to resize observer. Only return when requested and applies to all element.
<dael> astearns: Thanks chrishtr
<dael> chrishtr: I appriciate all the feedback
<dael> ken: Thanks for discussion
<dael> fantasai: Want to discuss optionality
<dael> astearns: Add an issue for that
TremayneChrist commented 5 years ago

So this will not be provided as a box option, but as a getter in the ResizeObserverEntry object?

If so, I see a problem with this when the scaling changes across multiple screens - The css pixel will stay the same, however, the device pixels will be different. This means that the resize observer will not fire, when it probably should?

https://jsbin.com/gamerul

kenrussell commented 5 years ago

It will be provided as a box option. The summary above didn't completely capture the discussion.

To summarize the conclusions from the meeting:

@chrishtr @atotic is this correct?

atotic commented 5 years ago

Yes. device-pixel-content-size will be an option, and the size will be reported on ResizeObserverEntry. Size could be defined as:

interface ResizeObserverSize {
    readonly attribute unrestricted double inlineSize;
    readonly attribute unrestricted double blockSize;
} ;

Side note: #3673 discussed how to handle fragments. Current resolution is that the sizes will be reported as an array. ResizeObserverEntry per spec would look like this:

{
    target: Element
    contentRect: {....}
    devicePixelContentSize: [{ inlineSize: ..., blockSize: ... }]
}
dlibby- commented 5 years ago

Addressed by #4476