Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] port the pattern-fill antialiasing fix from gl-js #4771

Merged
merged 1 commit into from
Apr 29, 2016

Conversation

mollymerp
Copy link
Contributor

@mollymerp mollymerp commented Apr 20, 2016

just like this

@mollymerp mollymerp added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Apr 20, 2016
@friedbunny friedbunny added the Core The cross-platform C++ core, aka mbgl label Apr 21, 2016
@mollymerp mollymerp force-pushed the outlinepattern-antialiasing branch 4 times, most recently from 47e9947 to 097cf73 Compare April 26, 2016 17:56
@mollymerp
Copy link
Contributor Author

mollymerp commented Apr 26, 2016

Render tests from Travis are here: https://mapbox.s3.amazonaws.com/mapbox-gl-native/render-tests/11966.1/index.html

It looks like this change is affecting more of the render tests than it should be. Render tests that I thought were being affected by this PR are actually consistent with the most recently merged PR build. The condition I'm using to see whether to apply the outline-pattern shader is if (properties.antialias && stroke_color == fill_color) {}.

@jfirebaugh mentioned in chat that I should try the conditional stroke_color[3] == -1 to test if the stroke-color property of a fill layer is not set, and if it isn't, apply the outlinepattern shader. That didn't seem to work (i.e. there was no change from the master branch in the rendering tests)

I will continue poking around, but if anyone has suggestions I'd love to hear them!

@jfirebaugh
Copy link
Contributor

@ansis Can you help @mollymerp here? 🙇

@ansis
Copy link
Contributor

ansis commented Apr 27, 2016

The render tests look right to me. The antialiasing doesn't match -js but that's expected because -js renders antialiasing wrong in headless testing: mapbox/mapbox-gl-js#2080

I think the failing outline test can be disabled in -native until we fix the antialising issue.

config.activeTexture = GL_TEXTURE0;
spriteAtlas->bind(true, glObjectStore);

setDepthSublayer(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be setDepthSublayer(2); like the non-pattern antialiasing call.

What this does is draws the outline at a further depth so that it doesn't get drawn on top of the inner part of the fill.

@ansis
Copy link
Contributor

ansis commented Apr 27, 2016

Once https://github.com/mapbox/mapbox-gl-native/pull/4771/files#r61337105 is fixed and tests failing because of antialiasing differences are disabled I think this should be good to merge.

@jfirebaugh do you still want to review anything here?

@jfirebaugh
Copy link
Contributor

LGTM!

@mollymerp mollymerp force-pushed the outlinepattern-antialiasing branch 4 times, most recently from 700fe38 to d3a7e7c Compare April 28, 2016 18:27
@mollymerp
Copy link
Contributor Author

mollymerp commented Apr 28, 2016

Updated the render test for test-Map.Offline that was failing:

old expected
screen shot 2016-04-28 at 11 28 17 am

new expected
screen shot 2016-04-28 at 11 28 41 am

thank you to @tmpsantos for helping me troubleshoot the failing builds! 💯

once everything is green, this is good to 🚢 @ansis ?

@mollymerp mollymerp force-pushed the outlinepattern-antialiasing branch from d3a7e7c to 11b544a Compare April 28, 2016 18:33
@mollymerp mollymerp removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Apr 28, 2016
@ansis
Copy link
Contributor

ansis commented Apr 28, 2016

🚢!

@mollymerp mollymerp changed the title [wip] port the pattern-fill antialiasing fix from gl-js [core] port the pattern-fill antialiasing fix from gl-js Apr 28, 2016
@mollymerp mollymerp force-pushed the outlinepattern-antialiasing branch 3 times, most recently from f6e0b98 to 6c7d31c Compare April 28, 2016 23:58
…to native

add outlinepattern shader class to relevant files

add outlinepattern code to painter_fill.cpp

add outlinepattern code to fill_bucket

refactor painter_fill, fix tests

fix merge conflicts and setDepthSublayer

update render test to no antialiasing so travis will be happy
@mollymerp mollymerp force-pushed the outlinepattern-antialiasing branch from 6c7d31c to c86f558 Compare April 29, 2016 00:13
@mollymerp mollymerp merged commit c86f558 into master Apr 29, 2016
@jfirebaugh jfirebaugh deleted the outlinepattern-antialiasing branch April 29, 2016 01:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants