Skip to content

RGB values need clamping #76

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.

Sign up for GitHub

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

Jump to bottom
Closed
larsbergstrom opened this issue Jun 25, 2015 · 6 comments
Closed

RGB values need clamping #76

larsbergstrom opened this issue Jun 25, 2015 · 6 comments

Comments

@larsbergstrom
Copy link
Contributor

Per http://www.w3.org/TR/CSS2/syndata.html#value-def-color, integers and percentages above the maximum should be clamped.

This breaks tests/wpt/css-tests/css21_dev/html4/background-color-088.htm and tests/wpt/css-tests/css21_dev/html4/background-color-089.htm (integers and percentages, respectively).
@SimonSapin
Copy link
Member

CSS 2 also says this, but the phrasing in CSS Color Level 4 is a bit clearer:

http://dev.w3.org/csswg/css-color/#rgb-functions

Some devices can output colors technically outside of the sRGB gamut, represented by channels with values less than 0% or greater than 100%. For this reason, values outside of the 0%-100% range are allowed, but are clamped to the device’s gamut.)

My understanding is that since this library doesn’t know the device’s gamut, it shouldn’t be doing the camping.

@SimonSapin
Copy link
Member

Given that, I’m not sure that these two tests are completely valid, but I don’t feel like arguing with CSSWG about that…

@SimonSapin SimonSapin mentioned this issue Aug 4, 2015
Merged
@SimonSapin
Copy link
Member

@nical, do you know if moz2d supports at all device gamuts other than 0% to 100% for R, G, and B?

@nical
Copy link

nical commented Aug 4, 2015

Moz2d doesn't attempt to do anything with device gamut as far as i know, but maybe some of the underlying backends do stuff under the hood I know that at least cairo clamps color values into the [0 - 1] range for instance.

@SimonSapin
Copy link
Member

Alright, let’s not bother trying to support non-standard gamuts, then.

SimonSapin added a commit to servo/servo that referenced this issue Aug 4, 2015
@SimonSapin
Upgrade cssparser
c6fc945 
Pick up the fix for servo/rust-cssparser#76
SimonSapin added a commit to servo/servo that referenced this issue Aug 4, 2015
@SimonSapin
Upgrade cssparser
a18b1ba 
Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.
@SimonSapin SimonSapin mentioned this issue Aug 4, 2015
Merged
bors-servo pushed a commit to servo/servo that referenced this issue Aug 4, 2015
Auto merge of #6957 - servo:cssparserup, r=larsbergstrom
d53e871 
Upgrade cssparser

Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.

r? @larsbergstrom

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6957)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this issue Aug 5, 2015
Auto merge of #6957 - servo:cssparserup, r=larsbergstrom
a909f72 
Upgrade cssparser

Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.

r? @larsbergstrom

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6957)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this issue Aug 5, 2015
Auto merge of #6957 - servo:cssparserup, r=larsbergstrom
1d180d1 
Upgrade cssparser

Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.

r? @larsbergstrom

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6957)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this issue Aug 5, 2015
Auto merge of #6957 - servo:cssparserup, r=larsbergstrom
31137cb 
Upgrade cssparser

Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.

r? @larsbergstrom

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6957)
<!-- Reviewable:end -->
SimonSapin added a commit to SimonSapin/servo that referenced this issue Aug 7, 2015
@SimonSapin
Upgrade cssparser
5d1a67c 
Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-
tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.
SimonSapin added a commit to servo/servo that referenced this issue Aug 7, 2015
@SimonSapin
Upgrade cssparser
8c5fee1 
Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-
tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.
SimonSapin added a commit to servo/servo that referenced this issue Aug 7, 2015
@SimonSapin
Upgrade cssparser
2f47830 
Pick up the fix for servo/rust-cssparser#76
bors-servo pushed a commit to servo/servo that referenced this issue Aug 7, 2015
Auto merge of #6957 - servo:cssparserup, r=larsbergstrom
eedd3a1 
Upgrade cssparser

Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.

r? @larsbergstrom

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6957)
<!-- Reviewable:end -->
SimonSapin added a commit to servo/servo that referenced this issue Aug 7, 2015
@SimonSapin
Upgrade cssparser
90d7d11 
Pick up the fix for servo/rust-cssparser#76
bors-servo pushed a commit to servo/servo that referenced this issue Aug 7, 2015
Auto merge of #6957 - servo:cssparserup, r=larsbergstrom+dzbarsky
9bd5291 
Upgrade cssparser

Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.

r? @larsbergstrom

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6957)
<!-- Reviewable:end -->
@SimonSapin
Copy link
Member

CSS Color Level 4 removed the paragraph quoted above about device gamuts outside of 0%...100%. Now it says to clamp:

https://drafts.csswg.org/css-color/#rgb-functions

Values outside these ranges are not invalid, but are clamped to the ranges defined here at computed-value time.

(The "ranges defined here" being 0...255 for unitless numbers and 0%...100% for percentages.)

jrmuizel pushed a commit to jrmuizel/gecko-cinnabar that referenced this issue Jun 12, 2017
@SimonSapin
servo: Merge #6957 - Upgrade cssparser (from servo:cssparserup); r=la…
8e7b2e5 
…rsbergstrom+dzbarsky

Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.

r? @larsbergstrom

Source-Repo: https://github.com/servo/servo
Source-Revision: 9bd5291aea24f5bc2780c864d207ce1496f8525f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Sep 30, 2019
@marco-c
servo: Merge #6957 - Upgrade cssparser (from servo:cssparserup); r=la…
1f899b6 
…rsbergstrom+dzbarsky

Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.

r? larsbergstrom

Source-Repo: https://github.com/servo/servo
Source-Revision: 9bd5291aea24f5bc2780c864d207ce1496f8525f

UltraBlame original commit: ea4db506c3695509fa14f50d4f6d991808d00291
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
@marco-c
servo: Merge #6957 - Upgrade cssparser (from servo:cssparserup); r=la…
86fed46 
…rsbergstrom+dzbarsky

Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.

r? larsbergstrom

Source-Repo: https://github.com/servo/servo
Source-Revision: 9bd5291aea24f5bc2780c864d207ce1496f8525f

UltraBlame original commit: ea4db506c3695509fa14f50d4f6d991808d00291
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
@marco-c
servo: Merge #6957 - Upgrade cssparser (from servo:cssparserup); r=la…
87e57bd 
…rsbergstrom+dzbarsky

Pick up the fix for servo/rust-cssparser#76

`*.ini` files removal based on running `./mach test-css tests/wpt/css-tests/css21_dev/html4/*color*`, I didn’t run the whole test suite.

r? larsbergstrom

Source-Repo: https://github.com/servo/servo
Source-Revision: 9bd5291aea24f5bc2780c864d207ce1496f8525f

UltraBlame original commit: ea4db506c3695509fa14f50d4f6d991808d00291
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nical @SimonSapin @larsbergstrom