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

fill blurring #424

Closed
wants to merge 4 commits into from
Closed

fill blurring #424

wants to merge 4 commits into from

Conversation

ansis
Copy link
Contributor

@ansis ansis commented Jun 4, 2014

Partially ports blurring from -native. Setting the following properties on a fill bucket blurs the fill:

{
    "prerender": true,
    "prerender-size": 1024,
    "prerender-blur": 1
}

prerender and prerender-size can't be functions yet, unlike native. I think we need some deeper thought reorganization here.

@mourner quick review?


for (var i = 0; i < passes; i++) {

console.log([1 / this.size, 0]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

console.log

@mourner
Copy link
Member

mourner commented Jun 4, 2014

Looks great. But prerender props should be specified in styles, not bucket, right? And what reorganization would we need to support functions? Are you concerned about performance here?

@ansis
Copy link
Contributor Author

ansis commented Jun 4, 2014

But prerender props should be specified in styles, not bucket, right?

Nope, these need to be on the bucket. It pre-renders a blurred texture for the tile once, and just linearly interpolates after that. This follows the -native implementation.

And what reorganization would we need to support functions?

Functions for buckets aren't supported yet

Are you concerned about performance here?

Drawing from the texture on each frame is fast. Its just the first frame that requires some extra processing. Its hard to measure carefully, but it feels fine. I'm not seeing any low-hanging optimizations, and performance concerns will be similar to those on -native.

@mourner
Copy link
Member

mourner commented Jun 4, 2014

@ansis currently the only reason bucket and style properties are separated is that bucket properties are needed when extracting data in a worker. This code doesn't have any worker calculations. Also, native style has them specified in styles: https://github.com/mapbox/mapbox-gl-native/blob/master/bin/style.js#L2070-L2072

@mourner mourner mentioned this pull request Jun 5, 2014
12 tasks
@mourner mourner added this to the v1.0.0 milestone Jun 5, 2014
@ansis
Copy link
Contributor Author

ansis commented Jun 5, 2014

@mourner you're right about -native having them in the style. I missed that

This feels odd to me. All other current style properties can be changed at render time without reprocessing the data, just by tweaking uniforms. Changing the prerender properties means prerendering the tile again. But as you mentioned, the buffers don't need to be reprocessed in the workers.

Also, in -native these properties are functions of the tile zoom, not the current zoom, unlike all style properties. What I'm wondering is how special cases like this should change how we deal with the style

@mourner mourner modified the milestones: future, v0.1.0 Jul 1, 2014
@yhahn yhahn closed this Jul 9, 2014
@yhahn
Copy link
Member

yhahn commented Jul 9, 2014

@ansis can you recreate this pull against the dev-pages branch?

@yhahn yhahn mentioned this pull request Jul 9, 2014
@jfirebaugh jfirebaugh deleted the texturerender branch July 23, 2014 00:03
bensleveritt pushed a commit to bensleveritt/mapbox-gl-js that referenced this pull request Oct 24, 2016
fix Draw.add validation and add tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants