Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Can't disable all sky rendering when terrain is enabled #4517

Closed
UberMouse opened this issue Aug 6, 2024 · 10 comments · Fixed by #4587 or #4607
Closed

Can't disable all sky rendering when terrain is enabled #4517

UberMouse opened this issue Aug 6, 2024 · 10 comments · Fixed by #4587 or #4607
Labels
bug Something isn't working PR is more than welcomed Extra attention is needed

Comments

@UberMouse
Copy link

maplibre-gl-js version: 4.5.1

browser: Vivaldi 6.8.3381.48

Steps to Trigger Behavior

  1. Open demonstration
  2. Disable sky with checkbox (this calls map.setSky(undefined))
  3. Notice the sky rendering turns off, but the horizon and fog colouring persists
  4. Disable terrain via the control in the top right, notice that horizon/fog colouring is now gone

Link to Demonstration

https://jsbin.com/hibilakuno/edit?html,output

Expected Behavior

The sky/horizon/fog rendering is disabled when terrain is enabled when calling setSky(undefined) or sky: undefined in a style spec

Actual Behavior

Only sky rendering is disabled when terrain is enabled

@HarelM
Copy link
Collaborator

HarelM commented Aug 6, 2024

It also seems like when trying to enable again the sky using the checkbox it doesn't bring them back.
It might be related to this code, but I'm not sure this is the only part that doesn't take sky as undefined correctly:

setSky(sky?: SkySpecification, options: StyleSetterOptions = {}) {

Feel free to investigate.

@HarelM HarelM added bug Something isn't working PR is more than welcomed Extra attention is needed labels Aug 6, 2024
@HarelM
Copy link
Collaborator

HarelM commented Aug 22, 2024

Should be fixed by #4587 and released shortly after this is merged.

@UberMouse
Copy link
Author

Just managed to check this out. Maybe I am misunderstanding something but this does not appear to fix the issue. I've attached two screenshots of the same strip of horizon, one with terrain enabled, one without terrain and otherwise identical. Both have sky disabled (via setStyle with an object that has sky: undefined), but there is a clear difference in hazyness near the horizon in the one with terrain on vs the one with terrain off. I would expect them both to look the same.

Terrain enabled
image

Terrain disabled
image

@HarelM
Copy link
Collaborator

HarelM commented Aug 27, 2024

Can you check how this looks before sky was added (version 4.0 for example)

@UberMouse
Copy link
Author

Yea here's what it looked like on 4.3.2

Terrain enabled
image

Terrain disabled
image

No haze

@HarelM
Copy link
Collaborator

HarelM commented Aug 27, 2024

Can you open a different issue then? Jsbin would be helpful to see the difference between versions and rendering.

@UberMouse
Copy link
Author

You want me to open the same issue again? I opened this issue to fix the bug I'm highlighting and it has a JSBin to enable switching the version to compare

@HarelM
Copy link
Collaborator

HarelM commented Aug 27, 2024

Sorry, my mistake, no need, I reread the original post...

@HarelM HarelM reopened this Aug 27, 2024
@HarelM
Copy link
Collaborator

HarelM commented Aug 27, 2024

There were multiple issues in this report I guess...

@HarelM
Copy link
Collaborator

HarelM commented Aug 27, 2024

Should be fixed in #4607.
I'm not sure if the fix should be in the shader itself, but I think this is good enough...

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working PR is more than welcomed Extra attention is needed
Projects
None yet
2 participants