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

Image resizing API #1367

Closed
glennw opened this issue Jun 12, 2017 · 7 comments
Closed

Image resizing API #1367

glennw opened this issue Jun 12, 2017 · 7 comments

Comments

@glennw
Copy link
Member

glennw commented Jun 12, 2017

Currently, once an image is created you cannot resize it. But this makes the API complicated to use when you need to resize (e.g. in the case of a canvas texture).

One option could be to pass a field on image creation that states if the image is immutable or may change size. That could give WR a hint to choose appropriate texture storage depending on the likely resize patterns of this image.

@glennw
Copy link
Member Author

glennw commented Jun 12, 2017

Thoughts @kvark ?

@asajeffrey
Copy link
Member

asajeffrey commented Jun 12, 2017

This came up in the context of servo/servo#17278.

@kvark
Copy link
Member

kvark commented Jul 10, 2017

e.g. in the case of a canvas texture

Canvas is sort of special. It's typically big enough to deserve it's own texture anyway, so that we can not bother caring about the resize in texture allocator. Are there any other prominent examples of the need for resize? If canvas is the only thing, then I think it would be simpler to treat everything as storage-immutable on the WR side and expect the ImageId to be re-generated for canvas resizes and such.

@asajeffrey
Copy link
Member

We are thinking of redoing the canvas implementation anyway (servo/servo#17497) which would probably make this change unnecessary.

@nical
Copy link
Contributor

nical commented Jul 11, 2017

Another use case is rendering blob images at the resolution of the screen while zooming. Currently blob images have a fixed resolution and we'd need to re-create the blob image as we zoom. Might not be extremely important, though, since I think that Firefox doesn't yet support async zoom on desktop (so we re-build the display list each frame during zoom, and can re-create the blob image along the way), but it would be nice to not have to rebuild the entire display list just because a blob image needed to get a new ImageKey in order to change its resolution when we ship webrender on android (nice but not necessary).

Considering how easy it is to support changing image sizes in the resource cache, I'd rather just support it there than move the complexity to the user of the API (and have to re build display lists just for that when we could avoid it).

@kvark
Copy link
Member

kvark commented Jul 12, 2017

@nical
Now that I look at it, there doesn't appear to be any fundamental problem of updating the same key with an entirely new data (new size/format). update_image_template would need to detect his and re-allocate the new space in texture cache accordingly.

bors-servo pushed a commit that referenced this issue Jul 19, 2017
Support resizing items in the texture cache.

I revived #925 to address #1367.

TextureCache::update now supports changing the size of the image by freeing and reallocating a rectangle in the cache.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1490)
<!-- Reviewable:end -->
@glennw
Copy link
Member Author

glennw commented Jul 21, 2017

This is resolved.

@glennw glennw closed this as completed Jul 21, 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

No branches or pull requests

4 participants