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

Make quadtrees generic #534

Closed
wants to merge 4 commits into from
Closed

Make quadtrees generic #534

wants to merge 4 commits into from

Conversation

@eschweic
Copy link

eschweic commented Jun 22, 2013

Rewrite quadtrees to store generic types rather than store points.
r? @pcwalton

origin: Point2D<f32>,
size: f32,
quadrants: [Option<~Quadtree<T>>, ..4],
scale: @mut f32,

This comment has been minimized.

@pcwalton

pcwalton Jun 22, 2013

Contributor

So I think instead of using an @mut here it might be better to have an &mut f32 threaded through each method call, and to remove scale from the Quadtree<T> datastructure.

This comment has been minimized.

@pcwalton

pcwalton Jun 22, 2013

Contributor

Actually, I noticed that the Quadtree doesn't mutate the scale at all. So you don't need a pointer at all, I don't think. Just have the caller of each method pass the scale as a plain old f32.

size: f32,
quadrants: [Option<~Quadtree<T>>, ..4],
scale: @mut f32,
max_tile_size: uint,

This comment has been minimized.

@pcwalton

pcwalton Jun 22, 2013

Contributor

OK, I see. Since you're making multiple Quadtree instances for each child, it seems like a waste to have max_tile_size duplicated. I think what you want is something like this:

pub struct Quadtree<T> {
    root: QuadtreeNode<T>,
    scale: f32,
    max_tile_size: uint,
}

struct QuadtreeNode<T> {
    tile: Option<T>,
    origin: Point2D<f32>,
    size: f32,
    quadrants: [Option<~QuadtreeNode<T>>, ..4],
}

For each method on each QuadtreeNode, have it take fields of its parent like so:

impl<T> QuadtreeNode<T> {
    fn munge(&mut self, scale: f32, max_tile_size: uint, ...) {
        ...
    }
}
pub struct Quadtree<T> {
tile: Option<T>,
origin: Point2D<f32>,
size: f32,

This comment has been minimized.

@pcwalton

pcwalton Jun 22, 2013

Contributor

I take it this the scale factor of each tile? Please add a comment :)

@eschweic
Copy link
Author

eschweic commented Jul 3, 2013

See #557 .

@eschweic eschweic closed this Jul 3, 2013
ChrisParis pushed a commit to ChrisParis/servo that referenced this pull request Sep 7, 2014
…ve-url-tests

Oops. Looks like I forgot to 'git add' this file in PR444.
glennw pushed a commit to glennw/servo that referenced this pull request Jan 16, 2017
[WIP] Simplify texture names and support more than two textures in the shaders.

WebRender currently mostly assumes two reusable texture slots: the color and mask textures, which are sometimes used as color and color, and will soon be also used for other purposes like the Y, U and V planes (requiring an extra slot).
The way some shaders refer to the mask texture (sMask) as an extra color texture is confusing, and while it is most often used as a mask, the fact that it is sometimes not used as such means one can never know for sure until careful review of the corresponding shader code.
We also need more samplers for some effects such as YUV images.

I propose that the reusable samplers have generic names (sTexture0, sTexture1, etc.) which are used in the generic places (for example the device mostly does not know whether a texture is used as a color, a mask or a YUV plane), with a collection of specific name aliases (COLOR_TEXTURE_0, MASK_TEXTURE, Y_TEXTURE, etc.) for the code manipulating the samplers for specific purposes. This applies to both the shaders and the various data structures generated in the render backend.

<!-- 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/534)
<!-- 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

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