whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.16k stars 2.69k forks source link

Add createImageBitMaps's imageOrientation "none" #8085

Open zcorpan opened 2 years ago

zcorpan commented 2 years ago

If it's web compatible, consider renaming the imageOrientation value "none" to "from-image" to match CSS's image-orientation property (and make it the default value), and add a new "none" that has the same semantics as CSS's none.

[...] CreateImageBitmap has an imageOrientation option. Unfortunately that spec and the CSS images level 3 spec were written without knowledge of each other, so we are in this awkward situation where passing {imageOrientation: "none"} to createImageBitmap is equivalent to {image-orientation: from-image} in CSS. createImageBitmap does not currently have an option that is equivalent to CSS's {image-orientation: none}, but it would be pretty easy to add that.

Before moving forward, I would like to add instrumentation to chromium to measure whether anyone is using {ImageOrientation: "none"} explicitly with createImageBitmap. If not, we could change the createImageBitmap spec to match the CSS semantics and change the default to "from-image", which would avoid breaking backwards compat for call sites that use the default. [...]

Originally posted by @junov in https://github.com/whatwg/html/issues/4495#issuecomment-1176695685

cc @whatwg/canvas

zcorpan commented 2 years ago

I ran this query in HTTP Archive. 0 results.

SELECT
  *
FROM (
  SELECT
    page,
    url,
    REGEXP_EXTRACT(body, r'(createImageBitmap\([^,\)]+,\s*{\s*imageOrientation\s*:\s*["\']none["\'])') AS match
  FROM
     `httparchive.response_bodies.2022_06_09_mobile`)
WHERE
  match IS NOT NULL

(I noticed a slight bug in the query: imageOrientation might not be the first member of options.)

There are matches in GitHub search, though.

junov commented 2 years ago

It looks like the only production code in the GitHub search to use imageOrientation:"none" is three.js and it's many clones. The rest of the hits are clones of web platform tests. This should be quite manageable. We can start by adding "from-image", making it synonymous with "none". And then deprecate "none". Once we are satisfied that usage of "none" is extinct, we can flip its meaning to match CSS. Does that sound reasonable?

On Thu, Jul 7, 2022, 4:45 a.m. Simon Pieters @.***> wrote:

I ran this query in HTTP Archive. 0 results.

SELECT FROM ( SELECT page, url, REGEXP_EXTRACT(body, r'(createImageBitmap([^,)]+,\s{\simageOrientation\s:\s*["\']none["\'])') AS match FROM httparchive.response_bodies.2022_06_09_mobile) WHERE match IS NOT NULL

(I noticed a slight bug in the query: imageOrientation might not be the first member of options.)

There are matches in GitHub search https://github.com/search?q=createimagebitmap+imageorientation&type=code, though.

— Reply to this email directly, view it on GitHub https://github.com/whatwg/html/issues/8085#issuecomment-1177263795, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAT4V6GPAMMWLIURGENKTNTVS2KI3ANCNFSM524RDIWQ . You are receiving this because you were mentioned.Message ID: @.***>

zcorpan commented 2 years ago

If the breakage seems manageable right now, I think a more aggressive approach could actually be less risk because there's less time between now and the desired end state in which new content can appear that depends on the current semantics of "none", which might not be manageable anymore (or at least, it could trend up instead of trending down). That is:

  1. ASAP implement and ship the renaming of "none" to "from-image". Maybe make usage of "none" throw an exception, which can inform developers about the change. Reach out to the known users and ask them to switch to "from-image".
  2. At least one release milestone later, implement and ship the new "none" value.

However, I think your suggested approach also sounds reasonable, but I would suggest outreach and at least a console message when using "none" in the transition period, and try to keep the transition period short.

junov commented 2 years ago

Okay, in terms of PRs against the spec should we start by renaming "none" to "from-image", and in a later PR, re-add "none" with the new semantics?

zcorpan commented 2 years ago

Sounds good to me.

kenrussell commented 2 years ago

From a web compatibility standpoint I think that throwing an exception upon use of imageOrientation: none is too aggressive - it would cause web pages to completely stop working, rather than potentially (not definitely) displaying incorrect results. A deprecation warning to the console would be more developer- and user-friendly.

Agree with trying to minimize the deprecation and switch-over period.

CC @mrdoob since the main use of this dictionary element is Three.js.

mrdoob commented 2 years ago

What would be the right way for adopting this without having to do user agent checks? We tend to just wait until the feature has been implemented in all major browsers.

junov commented 2 years ago

@mrdoob: Today, you could just remove imageOrientation: "none" everywhere. It won't change the code's behavior because that's the default anyways. Only keep "imageOrientation" in places where you set it to "flipY".

junov commented 2 years ago

I am currently writing a spec change for this, and I realized there are issues with how flipY is defined as well. I created a new issue for this: #8118

zcorpan commented 2 years ago

@junov I see a commit but no PR. Did you mean to open a PR?

mrdoob commented 2 years ago

@mrdoob: Today, you could just remove imageOrientation: "none" everywhere. It won't change the code's behavior because that's the default anyways. Only keep "imageOrientation" in places where you set it to "flipY".

Done!

smaug---- commented 2 years ago

@kdashg do you happen to have an opinion to this?

kdashg commented 2 years ago

Sounds like a good idea, and sounds like the webcompat story checks out, thanks all!

annevk commented 2 years ago

@junov it seems there's no PR here still.

yiyix commented 1 year ago

@annevk, PR is created: https://github.com/whatwg/html/pull/8710

zcorpan commented 1 year ago

PR was merged, closing.

zcorpan commented 1 year ago

Okay, in terms of PRs against the spec should we start by renaming "none" to "from-image", and in a later PR, re-add "none" with the new semantics?

I realized now that the second part hasn't happened yet. Reopening to add none with the new semantics. Also see https://github.com/whatwg/html/pull/8710#issuecomment-1379790963

hamishwillee commented 4 months ago

Does flipY respect EXIF data now, and will it after this change goes in?

I ask because the HTML spec explicitly states that it does not:

If the value of the imageOrientation member of options is "flipY", output must be flipped vertically, disregarding any image orientation metadata of the source (such as EXIF metadata), if any. [EXIF]

However the corresponding CSS spec indicates that flip follows from-image, which does respect EXIF.

So either the HTML spec is wrong and we have a plan to change flipY to match CSS flip, or it is right and we're not planning to match flipY to flip. Can you confirm the expected behaviour of flipY now and in future?

This is for MDN docs https://github.com/mdn/content/pull/34366

kenrussell commented 4 months ago

Multiple browser vendors discussed this a while back and (99% sure, from recollection) agreed to change createImageBitmap's behavior. The HTML spec should be updated to say: first EXIF orientation is applied, and then if imageOrientation is flipY, the image is flipped vertically. CC @kdashg @kkinnunen-apple @lexaknyazev @RafaelCintron

This behavior is tested in the WebGL conformance suite: https://github.com/KhronosGroup/WebGL/blob/main/sdk/tests/conformance/textures/misc/exif-orientation.html , runnable online here.

hamishwillee commented 4 months ago

Thanks very much @kenrussell . I've documented as "it should be". Hope you guys can tidy the spec to match too.