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

Support resizing items in the texture cache. #1490

Merged
merged 3 commits into from Jul 19, 2017
Merged

Conversation

@nical
Copy link
Collaborator

nical commented Jul 17, 2017

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.


This change is Reviewable

@nical
Copy link
Collaborator Author

nical commented Jul 17, 2017

r? @glennw

@nical nical force-pushed the nical:resize-image branch from 832ab26 to 2af8931 Jul 17, 2017
Copy link
Member

kvark left a comment

Nice! A few notes to address.

debug_assert_eq!(existing_item.allocated_rect.size.height, descriptor.height);
self.free_item_rect(existing_item.clone());

self.allocate_impl(

This comment has been minimized.

@kvark

kvark Jul 17, 2017

Member

this would allocate but not really fill it with data we got in the arguments (data), so I think return should be removed

let existing_item = self.items.get(image_id);
let existing_item = self.items.get(image_id).clone();

if existing_item.allocated_rect.size.width != descriptor.width ||

This comment has been minimized.

@kvark

kvark Jul 17, 2017

Member

we'd also want to compare the formats

@nical
Copy link
Collaborator Author

nical commented Jul 18, 2017

we'd also want to compare the formats

I left the assertion that formats don't change in update_image_template. It's probably easy to add but I would rather do it as a followup.

this would allocate but not really fill it with data we got in the arguments (data), so I think return should be removed

Oops! That and also I needed to discard the dirty rect when reallocating, and some remaining assertions that had to be removed. Fixed in the next commit which I'll squash whenever the we are done with the reviews.
I hacked together a little app to check that it works (and it does now). I'll have to figure out how properly test this. A good candidate for the rawtest framework!

@kvark
kvark approved these changes Jul 19, 2017
Copy link
Member

kvark left a comment

Looks like everything is good now, thanks!

@kvark
Copy link
Member

kvark commented Jul 19, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Jul 19, 2017

📌 Commit edada88 has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented Jul 19, 2017

Testing commit edada88 with merge 431f0d0...

bors-servo added a commit that referenced this pull request 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 -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 19, 2017

☀️ Test successful - status-travis
Approved by: kvark
Pushing 431f0d0 to master...

@bors-servo bors-servo merged commit edada88 into servo:master Jul 19, 2017
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.