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

Add devicePixelRatio property to PaintRenderingContext2D #446

Merged
merged 5 commits into from Sep 8, 2017

Conversation

xidachen
Copy link
Contributor

@xidachen xidachen commented Aug 7, 2017

Per discussion here: #436, CSS working group agreed to add a devicePixelRatio property to the PaintRenderingContext2D.

@bfgeek Please review.

Copy link
Contributor

@bfgeek bfgeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, @flackr we sure that we want it on the context?

@flackr
Copy link
Contributor

flackr commented Aug 9, 2017

No, in the discussion we resolved on adding it to the PaintWorkletGlobalScope so it would be consistent with devicePixelRatio.

@xidachen
Copy link
Contributor Author

xidachen commented Aug 9, 2017

I looked at the discussion history and you are right. I will make changes accordingly. Thanks.

@xidachen
Copy link
Contributor Author

@flackr @bfgeek Please review the new commit. Thank you.

Copy link
Contributor

@bfgeek bfgeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! @flackr ok?

@@ -131,10 +131,16 @@ partial interface CSS {

The {{PaintWorkletGlobalScope}} is the global execution context of the {{paintWorklet}}.

The {{PaintWorkletGlobalScope}} has a devicePixelRatio property, which represents the ratio
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can link to devicePixelRatio via. {{PaintWorkletGlobalScope/devicePixelRatio}}

@xidachen
Copy link
Contributor Author

@bfgeek added link to devicePixelRatio

The {{PaintWorkletGlobalScope}} has a {{PaintWorkletGlobalScope/devicePixelRatio}} property,
which represents the ratio of the resolution in physical pixels to the resolution of CSS
pixels for the current display device. When measured from the same display device, this
value must be identical to the Window.devicePixelRatio.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to ask to reference devicePixelRatio, but it looks to me like the csswg has resolved not to standardize devicePixelRatio, see also this blog post for unprefixing -webkit-device-pixel-ratio. If the working group hasn't changed their stance on this, perhaps it's worth a note that we should aim to track whatever the expected way of querying the display density from javascript is. That thread suggests that the standard way is resolution but as far as I can tell you're unable to query that from javascript.

@xidachen
Copy link
Contributor Author

@flackr Please review. The new commit now reference to {{Window/devicePixelRatio}}.

Copy link
Contributor

@flackr flackr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good with one adjustment.

The {{PaintWorkletGlobalScope}} has a {{PaintWorkletGlobalScope/devicePixelRatio}} property,
which represents the ratio of the resolution in physical pixels to the resolution of CSS
pixels for the current display device. When measured from the same display device, this
value must be identical to the Window.{{Window/devicePixelRatio}}.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we link to the spec which has a more precise definition of the device pixel ratio it probably makes sense to shorten this and not try to redefine what it means here. i.e. How about just The PaintWorkletGlobalScope has a devicePixelRatio property which is identical to the Window.devicePixelRatio property.

@xidachen
Copy link
Contributor Author

@flackr Thanks. I made the change, it is much simpler now :)

@xidachen
Copy link
Contributor Author

xidachen commented Sep 8, 2017

@bfgeek This can be merged

@bfgeek bfgeek merged commit 70d944c into w3c:master Sep 8, 2017
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.

None yet

3 participants