Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Give toDataURL() and toBlob() default parameter values #3477
Give toDataURL() and toBlob() default parameter values #3477
Changes from 3 commits
d0bcb74
c9f503f
b8bc6c3
e13b929
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should just use 1.0 here as well. @junov?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have reservations about this change. This is not backwards compatible and will cause regressions. Changing the default to 1.0 will cause all websites that currently use the default to suddenly create much larger files and to take more time than before. Currently the default is a sensible compromise between quality, compression ratio, and encoding performance. The reason the default value is not standardized is that different browsers use different encoder libraries that each have their own interpretation for this parameter. To overcome this problem, each browser gets to decide what its implementation-specific default should be. This provides web devs with a interoperable way of getting a reasonable quality/compression compromise.
In my opinion we should not specify a default parameter value unless we also standardize the interpretation of the parameter value. If we do go down that road, then we should set the default to something that is backwards compatible (not 1.0).
A way forward for the spec would be to select a reference open source implementation for each format that supports the quality parameter (e.g. libjpeg for image/jpeg), and to state that the interpretation of the quality parameter is the one defined by that reference implementation. Browsers that use a different encoder library may need to apply a conversion to the quality parameter in order to match the interpretation from the reference implementation. Then we could standardize the default parameter value in a backwards compatible way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If that's the case why did you specify a default for
OffscreenCanvas
? Is that a bug that needs to be fixed?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OffscreenCanvas is a new API, so there is no backward compatibility issue.
PNG, which is the default format, can support a variable quality setting, but historically PNG has been used for lossless compression and developers have an expectation that PNG encoding should be lossless by default, and many browser implementation ignore the quality parameter when encoding PNGs. That is why I set the default to 1.0.
I realize now that there still problem... there's the issue of providing an interoperable quality/compression compromise for JPEG. I guess this needs more thought...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds like we should say that for "image/png" we set quality to 1.0 irrespective of its actual value and for other formats the default is user agent defined (and we don't fiddle with the value unless it's out of range or not a number). And we remove the 1.0 default from IDL for OffscreenCanvas and not add it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see:
= 1.0
from IDL (including from theImageEncodeOptions
dictionary).convertToBlob()
(step 6.1).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would also be nice to have a non-normative note explaining that different implementations may have slightly different interpretations of "quality" and that when quality is not specified an implementation-specific default is used that represents a reasonable compromise between compression ratio, image quality and encoding time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an implementor, agree with @junov. A non-normative note would allow implementors some latitude e.g. to achieve said compromise in the case of the default. Was that added?
In the case on image/png, does opt_quality apply? If so, what does it mean given the image is losslessly encoded? It would be nice if opt_quality could be used to control the compression. 0 - minimal compression and so a fast encode. 1 - maximum compression and a slow encode. default - the implementation-specific compromise b/w compression & encoding time. Adios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://bugs.chromium.org/p/chromium/issues/detail?id=179289 Request to control compression quality for .toDataURL "image/png".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@noell that should be a separate issue as that would be a normative change to the standard (image/png has to ignore quality at the moment).