tangrams / refill-style

From Toner to Tangram
https://tangrams.github.io/refill-style
MIT License
34 stars 12 forks source link

Labels on landuse pattern getting cut off #31

Closed sensescape closed 7 years ago

sensescape commented 7 years ago

http://tangrams.github.io/refill-style/#8/36.671/-117.922

screen shot 2016-12-12 at 11 44 46 am
nvkelso commented 7 years ago

@bcamper Did this change recently?

bcamper commented 7 years ago

Not intentionally :) But could see it having been a casualty of either some optimizations we made to shader rendering (maybe a state change is being missed) or fixes to alpha blending. I'll look into it.

sensescape commented 7 years ago

Adding a stroke/halo to the text fixes it:

screen shot 2016-12-12 at 3 24 31 pm

but text looks like this sometimes, over landuse pattern:

screen shot 2016-12-12 at 3 25 54 pm screen shot 2016-12-12 at 3 27 07 pm
bcamper commented 7 years ago

@patriciogonzalezvivo I haven't looked at this yet, but for the bottom example, with text over the dot pattern: do you think the alpha is interacting weirdly with the shader perhaps? Could you see what alpha is output from the shader?

patriciogonzalezvivo commented 7 years ago

Weird... all this dot patterns don't have alpha: https://github.com/tangrams/refill-style/blob/gh-pages/refill-style.yaml#L1126-L1194 ... @sensescape do you know what layer or landuse style is interfering with what label layer ??

patriciogonzalezvivo commented 7 years ago

My apologize, @bcamper point that alpha is affected in this line https://github.com/tangrams/refill-style/blob/gh-pages/refill-style.yaml#L1141 I'm not sure why we were multiplying the alpha there... but I think doing this is better:

                color: |
                    color.rgb = mix(COLOR1, COLOR2, circle(tile(getTileCoords(),PATTERN_SCALE), DOT_SIZE));

The DOT_SIZE define should need to be adjusted.

Also as @bcamper point we should use a texture here... aastep which is use by circle was some performance cost...

Happy to introduce any of those changes: a. simplify the shader as is... b. replace it for a texture

Note: I feel that using a shader will be only justifiable if we want to apply a hill-shade effect using the dot size, like we did in TRON2.0

screen shot 2016-12-14 at 11 18 46 am

Also switching to tangram blocks will solve more of this kind of issues... some of the shader in this style are quite old. For example the above code have been improved before here http://tangrams.github.io/blocks/#polygons-dots

patriciogonzalezvivo commented 7 years ago

Another example of dots for hill-shading https://tangrams.github.io/tangram-sandbox/tangram.html?styles/callejas#16.764280621970464/37.79126/-122.46361

screen shot 2016-12-14 at 11 22 44 am

patriciogonzalezvivo commented 7 years ago

So... I fix the bug by avoiding using the alpha channel... this was the effect of making the dots a little bigger, because the alpha was "eating" the texture more than is colorated (alpha interpolation is different). Any how now DOT_SIZE need adjustments. @sensescape could you tweak it until makes sense for you.

Once that's done, and we have parameters we are happy with I can make texture of them : )

bcamper commented 7 years ago

@sensescape: @patriciogonzalezvivo put the fix on the gh-pages branch, so you should see it once this gets merged there (or you could merge it back here but may just make things more complicated):

https://github.com/tangrams/refill-style/commit/46b60c06b4acd53dbd003d7b950bc08ab349dafa

patriciogonzalezvivo commented 7 years ago

thanks! and sorry for that

bcamper commented 7 years ago

@sensescape I want to look into the issue with the California label (the original issue): can you show me the commit where you added the stroke, so I can compare before/after?

sensescape commented 7 years ago

@bcamper sure, it's here on this branch: https://github.com/tangrams/refill-style/tree/sensescape/places-refactor

sensescape commented 7 years ago

this is the default text stroke that fixed it: https://github.com/tangrams/refill-style/blob/sensescape/places-refactor/refill-style.yaml#L4108

sensescape commented 7 years ago

thanks! @patriciogonzalezvivo I'm tweaking dots pattern on places-refactor branch : )

bcamper commented 7 years ago

Actually it looks like the original problem was the same thing with weird alpha on the underlying dots shader, so it should also be fixed (if you want to remove the stroke later) by the same fix for that.

sensescape commented 7 years ago

ok! thanks : )

bcamper commented 7 years ago

Closing original issue, added #32 to optimize dot shaders to use textures instead as follow-up (which @patriciogonzalezvivo and I discussed).