Skip to content
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.

Sign up for GitHub

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

Self Masking for Vector Layers #584

Merged
merged 3 commits into from
Feb 1, 2024
Merged

Self Masking for Vector Layers #584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0f1b758
Add support for self-masking vector layers
maurhofer-ubique Jan 23, 2024
ae51246
Keep order of polygon/line geometries in vector layers, don't overrid…
maurhofer-ubique Jan 24, 2024
0d43778
adds iOS part
zimmermannubique Feb 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions android/src/main/cpp/graphics/objects/LineGroup2dOpenGl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,7 @@ void LineGroup2dOpenGl::render(const std::shared_ptr<::RenderingContextInterface

std::shared_ptr<OpenGlContext> openGlContext = std::static_pointer_cast<OpenGlContext>(context);
if (isMasked) {
if (isMaskInversed) {
glStencilFunc(GL_EQUAL, 0, 255);
} else {
glStencilFunc(GL_EQUAL, 128, 255);
}
glStencilFunc(GL_EQUAL, isMaskInversed ? 0 : 128, 255);
} else {
glEnable(GL_STENCIL_TEST);
glStencilMask(0xFF);
Expand Down
21 changes: 15 additions & 6 deletions android/src/main/cpp/graphics/objects/Polygon2dOpenGl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,23 @@ void Polygon2dOpenGl::render(const std::shared_ptr<::RenderingContextInterface>

std::shared_ptr<OpenGlContext> openGlContext = std::static_pointer_cast<OpenGlContext>(context);

GLuint stencilMask = 0;
GLuint validTarget = 0;
GLenum zpass = GL_KEEP;
if (isMasked) {
if (isMaskInversed) {
glStencilFunc(GL_EQUAL, 0, 255);
} else {
glStencilFunc(GL_EQUAL, 128, 255);
}
stencilMask += 128;
validTarget = isMaskInversed ? 0 : 128;
}
glStencilOp(GL_KEEP, GL_KEEP, GL_INCR);
if (renderPass.isPassMasked) {
stencilMask += 127;
zpass = GL_INCR;
}

if (stencilMask != 0) {
glStencilFunc(GL_EQUAL, validTarget, stencilMask);
glStencilOp(GL_KEEP, GL_KEEP, zpass);
Comment on lines +107 to +121
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The modifications to the render method for handling stencil operations based on masking conditions are well-intended for enhancing rendering flexibility. However, consider the following:

  • The logic for calculating stencilMask and validTarget should be thoroughly tested to ensure it behaves as expected under all combinations of isMasked and renderPass.isPassMasked.
  • The choice of 128 and 127 for modifying stencilMask values appears arbitrary without context. Ensure these values are documented to explain their significance and how they contribute to the intended stencil operations.
  • Using GL_INCR for zpass assumes a specific usage pattern of the stencil buffer. Confirm that this approach is compatible with the overall rendering strategy and does not risk stencil buffer overflow, which could lead to rendering issues.

Adding detailed comments to explain the logic behind the chosen values and operations would greatly aid in the maintainability of this code. Additionally, consider edge cases where the combined stencil operations might not produce the expected results and adjust or document as necessary.

}

glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);

drawPolygon(openGlContext, program, mvpMatrix);
Expand Down
16 changes: 14 additions & 2 deletions android/src/main/cpp/graphics/objects/PolygonGroup2dOpenGl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,21 @@ void PolygonGroup2dOpenGl::render(const std::shared_ptr<::RenderingContextInterf
if (!ready)
return;

GLuint stencilMask = 0;
GLuint validTarget = 0;
GLenum zpass = GL_KEEP;
if (isMasked) {
glStencilFunc(GL_EQUAL, isMaskInversed ? 0 : 128, 128);
glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
stencilMask += 128;
validTarget = isMaskInversed ? 0 : 128;
}
if (renderPass.isPassMasked) {
stencilMask += 127;
zpass = GL_INCR;
}

if (stencilMask != 0) {
glStencilFunc(GL_EQUAL, validTarget, stencilMask);
glStencilOp(GL_KEEP, GL_KEEP, zpass);
Comment on lines +101 to +115
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of stencil operations in the render method introduces a more flexible handling of masking conditions. However, consider the following points for improvement:

  • Ensure that the initial values of stencilMask and validTarget are appropriately chosen for all possible states of isMasked and renderPass.isPassMasked.
  • The increment of stencilMask by 127 when renderPass.isPassMasked is true might lead to unexpected behavior if both isMasked and renderPass.isPassMasked are true, as the total value could exceed the maximum value representable in the stencil buffer. Verify that this logic aligns with the intended stencil operations.
  • The use of GL_INCR for zpass when renderPass.isPassMasked is true assumes that the stencil buffer is being used in a way that supports incremental updates. Ensure this matches the overall rendering strategy and that there's no risk of overflow in the stencil values which could lead to visual artifacts.

Consider adding comments to explain the rationale behind the chosen stencil operation values, especially the significance of 128 and 127, to improve maintainability and readability for future developers.

}

std::shared_ptr<OpenGlContext> openGlContext = std::static_pointer_cast<OpenGlContext>(context);
Expand Down
16 changes: 14 additions & 2 deletions android/src/main/cpp/graphics/objects/PolygonPatternGroup2dOpenGl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,21 @@ void PolygonPatternGroup2dOpenGl::render(const std::shared_ptr<::RenderingContex

glUseProgram(program);

GLuint stencilMask = 0;
GLuint validTarget = 0;
GLenum zpass = GL_KEEP;
if (isMasked) {
glStencilFunc(GL_EQUAL, isMaskInversed ? 0 : 128, 128);
glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
stencilMask += 128;
validTarget = isMaskInversed ? 0 : 128;
}
if (renderPass.isPassMasked) {
stencilMask += 127;
zpass = GL_INCR;
}

if (stencilMask != 0) {
glStencilFunc(GL_EQUAL, validTarget, stencilMask);
glStencilOp(GL_KEEP, GL_KEEP, zpass);
}

prepareTextureDraw(program);
Expand Down
16 changes: 14 additions & 2 deletions android/src/main/cpp/graphics/objects/Quad2dInstancedOpenGl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,21 @@ void Quad2dInstancedOpenGl::render(const std::shared_ptr<::RenderingContextInter

glUseProgram(program);

GLuint stencilMask = 0;
GLuint validTarget = 0;
GLenum zpass = GL_KEEP;
if (isMasked) {
glStencilFunc(GL_EQUAL, isMaskInversed ? 0 : 128, 128);
glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
stencilMask += 128;
validTarget = isMaskInversed ? 0 : 128;
}
if (renderPass.isPassMasked) {
stencilMask += 127;
zpass = GL_INCR;
}

if (stencilMask != 0) {
glStencilFunc(GL_EQUAL, validTarget, stencilMask);
glStencilOp(GL_KEEP, GL_KEEP, zpass);
}

if (usesTextureCoords) {
Expand Down
16 changes: 14 additions & 2 deletions android/src/main/cpp/graphics/objects/Quad2dOpenGl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,21 @@ void Quad2dOpenGl::render(const std::shared_ptr<::RenderingContextInterface> &co

glUseProgram(program);

GLuint stencilMask = 0;
GLuint validTarget = 0;
GLenum zpass = GL_KEEP;
if (isMasked) {
glStencilFunc(GL_EQUAL, isMaskInversed ? 0 : 128, 128);
glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
stencilMask += 128;
validTarget = isMaskInversed ? 0 : 128;
}
if (renderPass.isPassMasked) {
stencilMask += 127;
zpass = GL_INCR;
}

if (stencilMask != 0) {
glStencilFunc(GL_EQUAL, validTarget, stencilMask);
glStencilOp(GL_KEEP, GL_KEEP, zpass);
Comment on lines +196 to +210
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The modifications to the render method in Quad2dOpenGl for handling stencil operations based on masking conditions are well-implemented. However, ensure that:

  • The logic for calculating stencilMask, validTarget, and zpass is thoroughly tested, especially considering the potential for both isMasked and renderPass.isPassMasked to be true simultaneously.
  • The values 128 and 127 used for stencilMask adjustments are clearly documented, explaining their significance and ensuring that they do not inadvertently exceed the stencil buffer's capacity.
  • The choice of GL_INCR for zpass is consistent with the overall rendering strategy and does not lead to stencil buffer overflow, which could cause rendering artifacts.

Adding comments to explain the rationale behind the chosen stencil operation values and ensuring that the implementation is robust against all possible combinations of masking conditions would improve code clarity and maintainability.

}

if (usesTextureCoords) {
Expand Down
16 changes: 14 additions & 2 deletions android/src/main/cpp/graphics/objects/Quad2dStretchedInstancedOpenGl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,21 @@ void Quad2dStretchedInstancedOpenGl::render(const std::shared_ptr<::RenderingCon

glUseProgram(program);

GLuint stencilMask = 0;
GLuint validTarget = 0;
GLenum zpass = GL_KEEP;
if (isMasked) {
glStencilFunc(GL_EQUAL, isMaskInversed ? 0 : 128, 128);
glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
stencilMask += 128;
validTarget = isMaskInversed ? 0 : 128;
}
if (renderPass.isPassMasked) {
stencilMask += 127;
zpass = GL_INCR;
}

if (stencilMask != 0) {
glStencilFunc(GL_EQUAL, validTarget, stencilMask);
glStencilOp(GL_KEEP, GL_KEEP, zpass);
}

if (usesTextureCoords) {
Expand Down
16 changes: 14 additions & 2 deletions android/src/main/cpp/graphics/objects/Text2dInstancedOpenGl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,21 @@ void Text2dInstancedOpenGl::render(const std::shared_ptr<::RenderingContextInter

glUseProgram(program);

GLuint stencilMask = 0;
GLuint validTarget = 0;
GLenum zpass = GL_KEEP;
if (isMasked) {
glStencilFunc(GL_EQUAL, isMaskInversed ? 0 : 128, 128);
glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
stencilMask += 128;
validTarget = isMaskInversed ? 0 : 128;
}
if (renderPass.isPassMasked) {
stencilMask += 127;
zpass = GL_INCR;
}

if (stencilMask != 0) {
glStencilFunc(GL_EQUAL, validTarget, stencilMask);
glStencilOp(GL_KEEP, GL_KEEP, zpass);
}

if (usesTextureCoords) {
Expand Down
16 changes: 14 additions & 2 deletions android/src/main/cpp/graphics/objects/Text2dOpenGl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,21 @@ void Text2dOpenGl::render(const std::shared_ptr<::RenderingContextInterface> &co
return;
}

GLuint stencilMask = 0;
GLuint validTarget = 0;
GLenum zpass = GL_KEEP;
if (isMasked) {
glStencilFunc(GL_EQUAL, isMaskInversed ? 0 : 128, 128);
glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
stencilMask += 128;
validTarget = isMaskInversed ? 0 : 128;
}
if (renderPass.isPassMasked) {
stencilMask += 127;
zpass = GL_INCR;
}

if (stencilMask != 0) {
glStencilFunc(GL_EQUAL, validTarget, stencilMask);
glStencilOp(GL_KEEP, GL_KEEP, zpass);
}

glUseProgram(program);
Expand Down
1 change: 1 addition & 0 deletions bridging/android/java/io/openmobilemaps/mapscore/shared/graphics/RenderPassConfig.kt
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions bridging/android/jni/graphics/NativeRenderPassConfig.cpp
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion bridging/android/jni/graphics/NativeRenderPassConfig.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions bridging/ios/MCRenderPassConfig+Private.mm
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions bridging/ios/MCRenderPassConfig.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions bridging/ios/MCRenderPassConfig.mm
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions djinni/graphics/core.djinni
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ render_pass_interface = interface +c +j +o {

render_pass_config = record {
render_pass_index: i32;
is_pass_masked: bool;
}

renderer_interface = interface +c +j +o {
Expand Down
34 changes: 34 additions & 0 deletions ios/graphics/Model/BaseGraphicsObject.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,38 @@ extension BaseGraphicsObject: MCGraphicsObjectInterface {
isMasked: isMasked,
screenPixelAsRealMeterFactor: screenPixelAsRealMeterFactor)
}

// MARK: - Stencil

func maskStencilState(readMask: UInt32 = 0b1111_1111, writeMask: UInt32 = 0b0000_0000) -> MTLDepthStencilState? {
let s = MTLStencilDescriptor()
s.stencilCompareFunction = .equal
s.stencilFailureOperation = .zero
s.depthFailureOperation = .keep
s.depthStencilPassOperation = .keep
s.readMask = readMask
s.writeMask = writeMask

let desc = MTLDepthStencilDescriptor()
desc.frontFaceStencil = s
desc.backFaceStencil = s

return device.makeDepthStencilState(descriptor: desc)
}

func renderPassMaskStencilState() -> MTLDepthStencilState? {
let s = MTLStencilDescriptor()
s.stencilCompareFunction = .equal
s.stencilFailureOperation = .keep
s.depthFailureOperation = .keep
s.depthStencilPassOperation = .incrementWrap
s.readMask = 0b1111_1111
s.writeMask = 0b0000_0001

let desc = MTLDepthStencilDescriptor()
desc.frontFaceStencil = s
desc.backFaceStencil = s

return device.makeDepthStencilState(descriptor: desc)
}
}
Loading
Loading