Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upMake it possible to tweak the maximum texture size. #877
Conversation
|
image.rs seems to be added as an empty file? |
webrender/src/device.rs
Outdated
| } | ||
| } | ||
|
|
||
| pub fn max_texture_size(&self) -> u32 { self.max_texture_size } |
This comment has been minimized.
This comment has been minimized.
glennw
Feb 15, 2017
Member
nit: There should be a new line here (unless there is something in the rust style guide specifically about this?).
webrender/src/resource_cache.rs
Outdated
| @@ -235,6 +235,8 @@ impl ResourceCache { | |||
| } | |||
| } | |||
|
|
|||
| pub fn max_texture_size(&self) -> u32 { self.texture_cache.max_texture_size() } | |||
This comment has been minimized.
This comment has been minimized.
webrender/src/renderer.rs
Outdated
| @@ -675,7 +675,12 @@ impl Renderer { | |||
| options.precache_shaders) | |||
| }; | |||
|
|
|||
| let mut texture_cache = TextureCache::new(); | |||
| let device_max_size = device.max_texture_size(); | |||
| let max_texture_size = options.max_texture_size | |||
This comment has been minimized.
This comment has been minimized.
glennw
Feb 15, 2017
Member
nit: This can avoid the map with something like:
cmp::min(device_max_size, options.max_texture_size.unwrap_or(device_max_size))
| @@ -556,16 +556,23 @@ pub struct AllocationResult { | |||
| } | |||
|
|
|||
| impl TextureCache { | |||
| pub fn new() -> TextureCache { | |||
| pub fn new(mut max_texture_size: u32) -> TextureCache { | |||
| if max_texture_size * max_texture_size > MAX_RGBA_PIXELS_PER_TEXTURE { | |||
This comment has been minimized.
This comment has been minimized.
webrender/src/texture_cache.rs
Outdated
| } | ||
| } | ||
|
|
||
| pub fn max_texture_size(&self) -> u32 { self.max_texture_size } |
This comment has been minimized.
This comment has been minimized.
|
Looks good, let's squash these and then r=me |
Merged
|
This seems to have an extra commit in it now? (The separate out debug/release builds commit). |
|
r=me once that stray commit is removed. |
|
@bors-servo r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Feb 20, 2017
Make it possible to tweak the maximum texture size. As a prelude to the work on supporting very large images, this pull request makes it possible to lower the maximum texture size. By default the maximum size is the one reported by GL and we can lower it with a setting in the RendererOptions (but not increase it). This should make it a lot easier to test support for large images, will give us the possibility to cap the maximum size for drivers that are too optimistic for their own sake, and also make it easier to reproduce bugs happening on hardware with lower capabilities. r? @glennw <!-- 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/877) <!-- Reviewable:end -->
|
|
bors-servo
added a commit
that referenced
this pull request
Feb 23, 2017
Tiled image support Here is a pretty straightforward implementation of image tiling which aims at being able to render images (and vector images) that are larger than the maximum texture size. The change mostly boils down to: - making it possible to upload textures with an offset in the source buffer - identifying individual tiles in the resource cache (See ```ImageRequest::tile```) - break up image display items into several primitives when the image is tiled The current limitation is that repeating images isn't supported with tiling yet. That will need to be fixed. It should only be a matter of extending the code in frame.rs so that it generate a set of image primitive for each repetition. I have been changing my mind regularly on the topic of whether tiling should be requested explicitly in the API or if it should be done somewhat automatically inside of WebRender. Currently I think that making this explicit in the API is simpler to implement and also simpler to test, so that's what I ended up implementing. It would probably make sense to also automatically tile images that are larger than the maximum texture size even if tiling isn't requested, because there isn't any way to render them otherwise. I don't have a particular attachment to the way I exposed it in the api (added ```RenderApi::add_image_with_tiling``` rather than passing an extra parameter in add_image to avoid changing the api), let me know if you have a preference. I am quite happy with how this works compared to what we have in gecko's layers, because tiles are uploaded on-demand which means we don't need to pay unnecessary cost for large images that are partially visible. The separation between the commits isn't too bad so I kept it this way in case it helps with the review, but don't hesitate to ask me to squash it into one commit before the review if you prefer. Also the PR is rebased on top of PR #877 which makes more code appear here than it should, but this will clear out when the latter PR lands. r? @glennw @kvark <!-- 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/897) <!-- Reviewable:end -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
nical commentedFeb 14, 2017
•
edited by larsbergstrom
As a prelude to the work on supporting very large images, this pull request makes it possible to lower the maximum texture size. By default the maximum size is the one reported by GL and we can lower it with a setting in the RendererOptions (but not increase it).
This should make it a lot easier to test support for large images, will give us the possibility to cap the maximum size for drivers that are too optimistic for their own sake, and also make it easier to reproduce bugs happening on hardware with lower capabilities.
r? @glennw
This change is