-
Notifications
You must be signed in to change notification settings - Fork 22
GPU Tessellation Tier 1 #900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
android/src/main/cpp/graphics/objects/Polygon2dTessellatedOpenGl.cpp
Outdated
Show resolved
Hide resolved
android/src/main/cpp/graphics/objects/Polygon2dTessellatedOpenGl.cpp
Outdated
Show resolved
Hide resolved
android/src/main/cpp/graphics/shader/TessellatedRasterShaderOpenGl.cpp
Outdated
Show resolved
Hide resolved
2969b7d to
97b17b7
Compare
android/src/main/cpp/graphics/objects/Quad2dTessellatedOpenGl.cpp
Outdated
Show resolved
Hide resolved
|
|
||
|
|
||
| RasterShaderOpenGl::RasterShaderOpenGl(bool projectOntoUnitSphere) | ||
| : programName(projectOntoUnitSphere ? "UBMAP_RasterShaderUnitSphereOpenGl" : "UBMAP_RasterShaderOpenGl") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: why does this shader have two variants if it doesn't use the value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea. Wondered it myself. I took it over from the other shaders and considered it a todo for the repo improvements issue.
|
|
||
| void Quad2dTessellatedOpenGl::computeGeometry(bool texCoordsOnly) { | ||
| // Data mutex covered by caller Quad2dTessellatedOpenGL::setup() | ||
| if (!texCoordsOnly) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style: split function (tex-coords part and geometry part).
Style 2: clearer as function, take input as param and return result, assign at call site vertices = computeVertexPositions(frame)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its currently like this because of consistency with the Quad2dOpenGl.cpp. But I could of course change both.
android/src/main/cpp/graphics/objects/Polygon2dTessellatedOpenGl.h
Outdated
Show resolved
Hide resolved
| bool Polygon2dOpenGl::isReady() { return ready; } | ||
|
|
||
| void Polygon2dOpenGl::setVertices(const ::SharedBytes & vertices_, const ::SharedBytes & indices_, const ::Vec3D & origin) { | ||
| void Polygon2dOpenGl::setSubdivisionFactor(int32_t factor) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be confusing that the tesselated / non-tesselated types are mixed behind the same interface. We need to manually keep track of where a tesselated object is used.
Would it be an option to have a separate type hierarchy for the tesselated types, or does that make integration much more complicated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree.
Thinking about it, the tessellated interfaces which would extend the normal ones would be almost empty though. The only change i did to the interfaces i think was the addition of the setSubdivisionFactor to the polygon interface. The reason i did this was for consistency with the quad interface, there for some reason the subdivision gets done on the level of the metal/opengl backend.
android/src/main/cpp/graphics/shader/TessellatedRasterShaderOpenGl.cpp
Outdated
Show resolved
Hide resolved
maurhofer-ubique
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I observed some strange precision issues on certain phones but don't have one with me at the moment. I'll try to find one to reproduce the issues.
android/src/main/cpp/graphics/objects/Polygon2dTessellatedOpenGl.h
Outdated
Show resolved
Hide resolved
shared/src/map/layers/tiled/vector/tiles/raster/Tiled2dMapVectorRasterTile.cpp
Outdated
Show resolved
Hide resolved
shared/src/map/layers/tiled/vector/Tiled2dMapVectorLayerConstants.h
Outdated
Show resolved
Hide resolved
2268b63 to
04fa9ba
Compare
04fa9ba to
dfd1d3b
Compare
🖼️ Image Diff Test FailureThe image comparison tests failed. Rendered images don't match the expected golden images. Investigate:
To approve the changes:
Last updated: Run #590 |
Improvements