felixpalmer / procedural-gl-js

Mobile-first 3D mapping engine with emphasis on user experience
https://www.procedural.eu/map
Mozilla Public License 2.0
1.29k stars 85 forks source link

Something is worng with elevation data #2

Closed gena closed 3 years ago

gena commented 3 years ago

The elevation is broken in all examples running on Win 10, 2080GPU, tried both chrome and firefox. image

felixpalmer commented 3 years ago

Thanks for the report. Could you paste the output you get from https://webglreport.com/?

gena commented 3 years ago

image


WebGL 2 Functions Implementation Status:
--
  | 88 of 88 new functions implemented.
  | copyBufferSubData
  | getBufferSubData
  | blitFramebuffer
  | framebufferTextureLayer
  | getInternalformatParameter
  | invalidateFramebuffer
  | invalidateSubFramebuffer
  | readBuffer
  | renderbufferStorageMultisample
  | texStorage2D
  | texStorage3D
  | texImage3D
  | texSubImage3D
  | copyTexSubImage3D
  | compressedTexImage3D
  | compressedTexSubImage3D
  | getFragDataLocation
  | uniform1ui
  | uniform2ui
  | uniform3ui
  | uniform4ui
  | uniform1uiv
  | uniform2uiv
  | uniform3uiv
  | uniform4uiv
  | uniformMatrix2x3fv
  | uniformMatrix3x2fv
  | uniformMatrix2x4fv
  | uniformMatrix4x2fv
  | uniformMatrix3x4fv
  | uniformMatrix4x3fv
  | vertexAttribI4i
  | vertexAttribI4iv
  | vertexAttribI4ui
  | vertexAttribI4uiv
  | vertexAttribIPointer
  | vertexAttribDivisor
  | drawArraysInstanced
  | drawElementsInstanced
  | drawRangeElements
  | drawBuffers
  | clearBufferiv
  | clearBufferuiv
  | clearBufferfv
  | clearBufferfi
  | createQuery
  | deleteQuery
  | isQuery
  | beginQuery
  | endQuery
  | getQuery
  | getQueryParameter
  | createSampler
  | deleteSampler
  | isSampler
  | bindSampler
  | samplerParameteri
  | samplerParameterf
  | getSamplerParameter
  | fenceSync
  | isSync
  | deleteSync
  | clientWaitSync
  | waitSync
  | getSyncParameter
  | createTransformFeedback
  | deleteTransformFeedback
  | isTransformFeedback
  | bindTransformFeedback
  | beginTransformFeedback
  | endTransformFeedback
  | transformFeedbackVaryings
  | getTransformFeedbackVarying
  | pauseTransformFeedback
  | resumeTransformFeedback
  | bindBufferBase
  | bindBufferRange
  | getIndexedParameter
  | getUniformIndices
  | getActiveUniforms
  | getUniformBlockIndex
  | getActiveUniformBlockParameter
  | getActiveUniformBlockName
  | uniformBlockBinding
  | createVertexArray
  | deleteVertexArray
  | isVertexArray
tdurand commented 3 years ago

I have the same problem with Windows 10 : Chrome and firefox .. but when I try with Ubuntu and chromium it works fine.

BTW 🎉🎉 for releasing this.. I have been following your work with procedural.eu the past years ;-)

felixpalmer commented 3 years ago

Unfortunately I don't have a Windows 10 machine to test on, but I do have a theory about why this is happening. Could you try and see if the problem persists when you add the interpolateFloat GET parameter? I.e. https://felixpalmer.github.io/procedural-gl-js/?interpolateFloat

tdurand commented 3 years ago

The problem seems to be the same.. let me know if I can do another test (can be pulling the repo and changing a few things in the code also if you need to)

felixpalmer commented 3 years ago

@tdurand thanks for the offer, I don't have any other obvious suggestion, debugging these issues can be difficult as GPUs are black boxes and it is hard to know what exactly is going wrong. Add to this the fact that this is going through ANGLE and the debugging process can be "fun" (e.g. https://stackoverflow.com/questions/27348125/colors-output-from-webgl-fragment-shader-differ-significantly-across-platforms)

The terrain lookup is done via two (dependent) texture lookups, see shaders/getHeight.glsl and shaders/readTexVirtual.glsl for details. One thing to notice is the manual bilateral filtering that is done in shaders/readTexVirtual.glsl. My best guess would be that the issue is somewhere in this code, so if this code makes sense to you, you could try playing around with it and see if anything "interesting" happens. Things I would try:

tdurand commented 3 years ago

I see.. yes I think the best way for you would be to get your hands on a machine running windows 10 and reproduce the problem (not sure actually if it happens on every windows machine ?)

I did tried

felixpalmer commented 3 years ago

Well, Paris does look a lot flatter than the image posted by gena.

As for the dev flow, changing the value in getHeight should be very noticeable. Running npm run dev is probably not going to work on Windows, try npm run build-dev and then be sure to open the index.module.html file (as this will include the dev build. But yes, you are right it is likely best if I try it on a Windows machine myself

gena commented 3 years ago

Just tried npm run build-dev with index.module.html - no luck, similar errors, looks indeed as some compatibility issue.

tdurand commented 3 years ago

Yes I wasn't getting the updated results.. now using index.module.html after a npm run build-dev and it does reflects the changes in the code:

highp

zoomed in smoothwave

zoomed in smoothwaweindex

zoom out smoothwaweindexzoomout

From what I understand.. the general elevation value is respected.. but it does this staircase things on zoom in .. the highp doesn't fix it

Hope this is helpful + yes I guess next step is for you to get your hands on a windows 10 laptop...

Happy to help with further test !

Thibault

felixpalmer commented 3 years ago

@tdurand OK that is surprising, but does seem to narrow down the issue. I was expecting those sin curves to come out smooth. I tried launching a Window VM via BrowserStack but it didn't work. Would you mind also trying the following alternative return statements:

tdurand commented 3 years ago

All of them seem to come out smooth , I must admit I don't understand why 😁 , but 👍

return 1000.0 * sin( 25.0 * 1024.0 * indirectionUv.x );

1test

return 1000.0 * sin( 32.0 * tile.x );

2test

return 1000.0 * sin( 0.001 * p.x );

3test

Pomax commented 3 years ago

@felixpalmer Note that windows 10 is technically entirely free to use (you just can't customize it until you activate it), so you should be able to just grab the installer and set up a dual boot if you want to better test this.

felixpalmer commented 3 years ago

I managed to reproduce the bug and pushed a fix in https://github.com/felixpalmer/procedural-gl-js/pull/5

@gena @tdurand could you try and see if this resolves the issue for you?

tdurand commented 3 years ago

Fixed !

felixpalmer commented 3 years ago

@tdurand @gena thank you both for help in debugging!