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

Use naturalSize doesn't make sense for css images #98

Closed
sefeng211 opened this issue Apr 19, 2022 · 10 comments · Fixed by #99
Closed

Use naturalSize doesn't make sense for css images #98

sefeng211 opened this issue Apr 19, 2022 · 10 comments · Fixed by #99

Comments

@sefeng211
Copy link

We get the idea that the spec uses naturalSize for reporting the actual rendered size of the images, however the naturalSize concept doesn't make sense for CSS images.

And there are many things to effect the final rendered size of a CSS image, and getting the intrinsic size doesn't mean that's the rendered size.

I think we should need to define a better description for CSS images.

cc @emilio

@yoavweiss
Copy link
Contributor

Thanks for reporting!
I agree that getting the naturalWidth and naturalHeight through the getters is very <img> specific, and that processing model makes no sense for background images.

The point of using the naturalSize to displaySize ratio is one that I think still makes sense for background images, although the parameters that go into the displaySize may be more complex, as you mention. Do you know if CSS background images have such concepts that we can point at directly?

@sefeng211
Copy link
Author

Emilio, do you have suggestions of what the spec should be? @emilio

@emilio
Copy link

emilio commented Apr 20, 2022

I mean, there's a lot that can influence how a background image is displayed (background size, background-repeat, other backgrounds occluding it...). What's the intention of the displaySize? Presumably the "rendered size" of the image? If so something like this plus accounting for repeat and so could do...

@yoavweiss
Copy link
Contributor

The intention is indeed the "rendered size".

@noamr
Copy link
Contributor

noamr commented Apr 25, 2022

I think the natural/intrinsic size still makes sense for background images, and is not different from a regular img that's displayed with custom width/height.
A large photo and a repeating tile with some pattern should behave differently.

@emilio
Copy link

emilio commented Apr 25, 2022

How does it make sense? It makes no sense to me that background-image: url(giant-image.png); background-size: 10px; would report giant display size.

@noamr
Copy link
Contributor

noamr commented Apr 25, 2022

How does it make sense? It makes no sense to me that background-image: url(giant-image.png); background-size: 10px; would report giant display size.

How is that different from img src=giant.png width=1 height=1 or img src=giant.png style="transform: scale(.000001)">?

@emilio
Copy link

emilio commented Apr 25, 2022

Well, right, the spec as-is would return 1x1 for at least your first example wouldn't it? You're right it wouldn't for the second tho.

@noamr
Copy link
Contributor

noamr commented Apr 25, 2022

Well, right, the spec as-is would return 1x1 for at least your first example wouldn't it? You're right it wouldn't for the second tho.

Or to other examples like object-size. This is actually not an easy problem to solve, I am merely stating that it's not specific to css images.

@noamr
Copy link
Contributor

noamr commented Apr 26, 2022

Researching this a bit, I think we can use the CSS terms concrete object size and natural dimensions, which apply to regular images and to background images, as well as to video etc.

noamr added a commit that referenced this issue Apr 26, 2022
This takes CSS sizing and layout for backgrounds & regular images into account.

Closes #98
@noamr noamr closed this as completed in #99 Jul 19, 2022
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 a pull request may close this issue.

4 participants