tangrams / refill-style

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

add shaded relief and no-texture themes #82

Closed sensescape closed 6 years ago

sensescape commented 6 years ago

fixes #50.

Expecting final tweaks thru next week in NYC, with in person review by @bcamper and @meetar then.

nvkelso commented 6 years ago

@matteblair @tallytalwar Can you verify this on ES, please? It's largely the same as Walkabout so it "should just work".

@bcamper Can you take particular attention to https://github.com/tangrams/refill-style/pull/82#discussion_r154246404, please?

@meetar Does it all look good to you now you spent time in NYC reviewing with @sensescape?

I'd like to wrap a bow around development this week so release management and blog post announcement can go out end of next week.

matteblair commented 6 years ago

@nvkelso Can I try out these new themes on the web somewhere to compare the outputs? (edit: I figured out how to just run it locally)

tallytalwar commented 6 years ago

I am seeing shader compilation failure with terrain-pattern imports on ES. Will continue debugging that tomorrow.

Edit fixed here: b7ad10d (@matteblair mentioned thats a difference between how JS and ES interpret shader injected defines, JS always inject these defines as float, whereas ES has a liking for being more strict here :D)

nvkelso commented 6 years ago

Need decision if this is version 9.1.0 or 10.0.0

nvkelso commented 6 years ago

Please remove:

nvkelso commented 6 years ago

Besides my comments today above, the code is looking good to me. I'll have another visual pass later this week, too.

nvkelso commented 6 years ago

Is there an SDK global for turning the coastline texture on and off, please?

bcamper commented 6 years ago

Made some comments about streamlining dashed styles since those can now be configured directly within draw groups... other than that, I think it's OK, but it's hard to tell because it's a biiiig diff :)

bcamper commented 6 years ago

@meetar did we never add that to docs? could swear we at least had it in a branch :)

On Tue, Dec 12, 2017 at 5:42 PM, Geraldine Sarmiento < notifications@github.com> wrote:

@sensescape commented on this pull request.

In refill-style.yaml https://github.com/tangrams/refill-style/pull/82#discussion_r156517455:

@@ -716,17 +583,20 @@ styles: dash: [0.25, 1.0] dash_background_color: global.offwhite_color

  • region_dash:

I found it, but it's not in the documentation

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tangrams/refill-style/pull/82#discussion_r156517455, or mute the thread https://github.com/notifications/unsubscribe-auth/AABBXbt2nztHAf5f6G2XDdIKf9Ufeg_sks5s_wFNgaJpZM4QxuDx .

tallytalwar commented 6 years ago

dash is there is docs: https://github.com/tangrams/tangram-docs/blob/gh-pages/pages/draw.md#dash. I was playing with this yesterday, while reviewing tangram-es code for documentation.

nvkelso commented 6 years ago

Ready to rock and roll.