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

Automatic tile removal #591

Merged
merged 1 commit into from Jul 17, 2013
Merged

Automatic tile removal #591

merged 1 commit into from Jul 17, 2013

Conversation

@eschweic
Copy link

eschweic commented Jul 16, 2013

The quadtree is now initialized with a memory limit, and tiles are deleted automatically if that limit is exceeded. The memory limit can also be set to None to prevent this behavior.

pub fn new(x: uint, y: uint, width: uint, height: uint, tile_size: uint) -> Quadtree<T> {
/// Takes in the initial width and height of the space, a maximum tile size, and
/// a maximum amount of memory. Tiles will be deleted if this memory is exceeded.
/// Set max_mem to 0 to turn off automatic tile removal.

This comment has been minimized.

@pcwalton

pcwalton Jul 16, 2013

Contributor

I think I might prefer Option<uint>... it makes it easier to avoid forgetting the check against zero.

/// The interface used by the quadtree to get info about LayerBuffers
pub trait Tile {
/// Returns the amount of memory used by the tile
fn get_size(&self) -> uint;

This comment has been minimized.

@pcwalton

pcwalton Jul 16, 2013

Contributor

Maybe call this get_mem() instead?

/// a bool that is true if the child has no tiles and needs to be deleted, and an integer showing the
/// amount of memory changed by the operation. Unfortunately, the tile has to be an option, because
/// there are occasionally leaves without tiles. However, the option will always be Some as long as
/// the quadtree is not empty.

This comment has been minimized.

@pcwalton

pcwalton Jul 16, 2013

Contributor

As long as the quadtree node is not empty, right?

@@ -171,7 +193,8 @@ impl<T> QuadtreeNode<T> {

/// Add a tile associated with a given position in page coords. If the tile size exceeds the maximum,
/// the node will be split and the method will recurse until the tile size is within limits.
fn add_tile(&mut self, x: f32, y: f32, tile: T, tile_size: f32) {
/// Returns an int that represents change in tile memory.

This comment has been minimized.

@pcwalton

pcwalton Jul 16, 2013

Contributor

I might say "returns the difference in tile memory between the new quadtree node and old quadtree node"

/// and a redisplay boolean. See QuadTree function description for more details.
fn get_tile_rects(&mut self, window: Rect<f32>, valid: &fn(&T) -> bool, scale: f32, tile_size: f32) ->
(~[BufferRequest], bool) {
/// Also returns an int that represents change in tile memory.

This comment has been minimized.

@pcwalton

pcwalton Jul 16, 2013

Contributor

I'm confused, why would getting the tile rect change the tile memory?

bors-servo pushed a commit that referenced this pull request Jul 17, 2013
The quadtree is now initialized with a memory limit, and tiles are deleted automatically if that limit is exceeded. The memory limit can also be set to None to prevent this behavior.
@pcwalton

This comment has been minimized.

Copy link

pcwalton commented on 5468885 Jul 17, 2013

r+

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented on 5468885 Jul 17, 2013

saw approval from pcwalton
at eschweic@5468885

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jul 17, 2013

merging eschweic/servo/del-tiles = 5468885 into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jul 17, 2013

eschweic/servo/del-tiles = 5468885 merged ok, testing candidate = c17ede3

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jul 17, 2013

fast-forwarding master to auto = c17ede3

This comment has been minimized.

Copy link

aydinkim replied Jul 29, 2013

As I tested this commit, It may exists some memory leakage at the time of tile removal.

This comment has been minimized.

Copy link

eschweic replied Jul 29, 2013

@aydinkim Textures seem to be deleting correctly according to the OpenGL Profiler. Where do you see the leak?

bors-servo pushed a commit that referenced this pull request Jul 17, 2013
The quadtree is now initialized with a memory limit, and tiles are deleted automatically if that limit is exceeded. The memory limit can also be set to None to prevent this behavior.
@bors-servo bors-servo merged commit 5468885 into servo:master Jul 17, 2013
1 check passed
1 check passed
default all tests passed
ChrisParis pushed a commit to ChrisParis/servo that referenced this pull request Sep 7, 2014
Wrong order for some nodes unrelated to shadow dom
glennw pushed a commit to glennw/servo that referenced this pull request Jan 16, 2017
Add specific shaders for rectangles with CLIP feature.

Having the base rectangle shader include clip makes the software
mesa implementation (somewhat) slower for reftests. This is enough
of a timing difference that it triggers a lot of intermittent
test failures in Servo.

Instead, include a simple rectangle shader for workloads that
don't have a clip mask.

The other shaders are typically more expensive and not used as
frequently by the reftests, so hopefully this change is enough
to work around the Servo intermittent bugs.

<!-- 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/591)
<!-- Reviewable:end -->
@jdm jdm mentioned this pull request Nov 5, 2019
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

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