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

Change texture creation to use nearest filtering. Fixes several reftests #91

Merged
merged 1 commit into from Jul 29, 2014

Conversation

@glennw
Copy link
Member

glennw commented Jul 29, 2014

due to the different texture sizes that are created now with the removal
of the quadtree code.

@zwarich
Copy link
Contributor

zwarich commented Jul 29, 2014

Where does this actually matter? Is zooming still going to be using bilinear?

@glennw
Copy link
Member Author

glennw commented Jul 29, 2014

Several of the reftests fail on my machine without this change - due to a single pixel border at the edge of the tile getting some bilinear filtering. This seems to be due to the way tiles are different sizes since the removal of the quadtree. I can dig a bit deeper and see if we can get pixel perfect blitting though, so we can retain the bilinear filtering, if we do need it?

@zwarich
Copy link
Contributor

zwarich commented Jul 29, 2014

We should probably modify the renderer to use nearest when there is a scale of 1 and use linear otherwise.

@glennw
Copy link
Member Author

glennw commented Jul 29, 2014

rendergl.rs Outdated
@@ -530,7 +542,8 @@ impl Render for Tile {
}
}

pub fn render_scene<T>(root_layer: Rc<Layer<T>>, render_context: RenderContext, scene: &Scene<T>) {
pub fn render_scene<T>(root_layer: Rc<Layer<T>>, render_context: RenderContext,
scene: &Scene<T>, has_scale: bool) {

This comment has been minimized.

@zwarich

zwarich Jul 29, 2014

Contributor

Why doesn't it suffice to get the scale from the Scene's transform?

This comment has been minimized.

@glennw

glennw Jul 29, 2014

Author Member

We could do, but it's often the case that with FP accuracy the scale isn't exactly 1.0 by the time it gets to the transform matrix, so it seems safer to check it earlier.

I can change it to just extract it from the matrix though if you'd prefer.

This comment has been minimized.

@zwarich

zwarich Jul 29, 2014

Contributor

I'd rather just extract it from the matrix. If that doesn't work in our testing config, then we probably have another bug.

Down the road if we have to do more tests of transforms we'll probably have to adopt the slightly disappointing solution of a fixed epsilon value.

This comment has been minimized.

@glennw

glennw Jul 29, 2014

Author Member

Updated to extract from matrix.

rendergl.rs Outdated
@@ -415,6 +415,16 @@ pub fn bind_and_render_quad(render_context: RenderContext,

use_program(program_id);
active_texture(TEXTURE0);

let has_scale = transform.m11 as uint != texture.size.width ||
transform.m22 as uint != texture.size.height;

This comment has been minimized.

@zwarich

zwarich Jul 29, 2014

Contributor

Technically speaking, don't we want to check that transform is a 2D affine transform and there is no skew or nonrectilinear rotation? I'd feel cruel asking you to implement this, given that it doesn't matter right now, but can you at least add a FIXME to that effect?

This comment has been minimized.

@glennw

glennw Jul 29, 2014

Author Member

Yep - which can make it tricky to extract the scale exactly. But I've added a FIXME which will do for now.

zwarich pushed a commit that referenced this pull request Jul 29, 2014
Change texture creation to use nearest filtering. Fixes several reftests
@zwarich zwarich merged commit b5a4884 into servo:master Jul 29, 2014
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.