Add support for stripes in colored polygons

[maurhofer-ubique]

Summary by CodeRabbit

• New Features
  • Introduced striped polygon rendering capabilities across platforms.
  • Added support for dynamic shader creation based on the striped flag.
  • Implemented a new stripeWidth parameter for polygon vector styles, enabling the configuration of stripe widths.
• Enhancements
  • Improved rendering performance by optimizing shader parameter handling.
  • Enhanced visual styling options for polygon layers with the addition of stripes.
• Bug Fixes
  • Fixed issues related to polygon rendering scale adjustments.
• Refactor
  • Updated method signatures across various components to support striped rendering.
• Documentation
  • Added comments and documentation related to new striped polygon features and shader adjustments.

[coderabbitai]

Walkthrough

The overarching change introduces a new feature for rendering striped polygons across various platforms, including Android and iOS. It involves adding a isStriped parameter to control shader behavior, a scaleFactorHandle for scaling in OpenGL, and modifications to shader creation and handling in both native and bridged code. This update spans across the graphics and shader components, enhancing the flexibility of polygon rendering by allowing for striped patterns and scaling adjustments.

Changes

File Path	Change Summary
.../cpp/graphics/objects/PolygonGroup2dOpenGl.*	Added scaleFactorHandle for scaling factors in rendering.
.../cpp/graphics/shader/ColorPolygonGroup2dShaderOpenGl.*	Constructor now takes isStriped to initialize shaders accordingly.
.../cpp/graphics/shader/ShaderFactoryOpenGl.*	createPolygonGroupShader now includes isStriped parameter.
.../jni/graphics/shader/NativeShaderFactoryInterface.* bridging/.../shader/ShaderFactoryInterface.kt	Modified createPolygonGroupShader to accept isStriped parameter.
bridging/ios/MCShaderFactoryInterface+Private.mm bridging/ios/MCShaderFactoryInterface.h	Updated method to take isStriped for iOS bridging.
djinni/graphics/shader/shader.djinni	Updated interface to include is_striped in method signature.
ios/graphics/Model/Polygon/PolygonGroup2d.swift ios/graphics/Pipelines/PipelineLibrary.swift ios/graphics/Shader/Metal/PolygonGroupShader.metal ios/graphics/Shader/PolygonGroupShader.swift ios/graphics/Shader/ShaderFactory.swift	iOS specific updates for handling isStriped and related rendering adjustments.
shared/public/PolygonVectorLayerDescription.h shared/public/ShaderFactoryInterface.h	Added stripeWidth and updated method signatures to include isStriped.
.../map/layers/tiled/vector/...	Adjustments for isStriped and stripeWidth in vector tile parsing and rendering.

"In the world of code and light,

Where polygons dance, both day and night,

A rabbit hopped, with stripes in sight,

Crafting shades and scales so bright. 🎨🐇"

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository from git and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[zimmermannubique]

@maurhofer-ubique auch hier nun iOS drin.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 771f29d and 1844d06.

Files selected for processing (23)

• android/src/main/cpp/graphics/objects/PolygonGroup2dOpenGl.cpp (2 hunks)
• android/src/main/cpp/graphics/objects/PolygonGroup2dOpenGl.h (1 hunks)
• android/src/main/cpp/graphics/shader/ColorPolygonGroup2dShaderOpenGl.cpp (2 hunks)
• android/src/main/cpp/graphics/shader/ColorPolygonGroup2dShaderOpenGl.h (2 hunks)
• android/src/main/cpp/graphics/shader/ShaderFactoryOpenGl.cpp (1 hunks)
• android/src/main/cpp/graphics/shader/ShaderFactoryOpenGl.h (1 hunks)
• bridging/android/java/io/openmobilemaps/mapscore/shared/graphics/shader/ShaderFactoryInterface.kt (2 hunks)
• bridging/android/jni/graphics/shader/NativeShaderFactoryInterface.cpp (2 hunks)
• bridging/android/jni/graphics/shader/NativeShaderFactoryInterface.h (2 hunks)
• bridging/ios/MCShaderFactoryInterface+Private.mm (2 hunks)
• bridging/ios/MCShaderFactoryInterface.h (1 hunks)
• djinni/graphics/shader/shader.djinni (1 hunks)
• ios/graphics/Model/Polygon/PolygonGroup2d.swift (4 hunks)
• ios/graphics/Pipelines/PipelineLibrary.swift (5 hunks)
• ios/graphics/Shader/Metal/PolygonGroupShader.metal (3 hunks)
• ios/graphics/Shader/PolygonGroupShader.swift (2 hunks)
• ios/graphics/Shader/ShaderFactory.swift (1 hunks)
• shared/public/PolygonVectorLayerDescription.h (2 hunks)
• shared/public/ShaderFactoryInterface.h (1 hunks)
• shared/src/map/layers/tiled/vector/Tiled2dMapVectorLayerParserHelper.cpp (1 hunks)
• shared/src/map/layers/tiled/vector/sublayers/background/Tiled2dMapVectorBackgroundSubLayer.cpp (1 hunks)
• shared/src/map/layers/tiled/vector/tiles/polygon/Tiled2dMapVectorPolygonTile.cpp (3 hunks)
• shared/src/map/layers/tiled/vector/tiles/polygon/Tiled2dMapVectorPolygonTile.h (1 hunks)

Additional comments: 45

android/src/main/cpp/graphics/shader/ShaderFactoryOpenGl.h (1)

• 28-28: The method signature for createPolygonGroupShader correctly includes the bool isStriped parameter as intended by the PR objectives.

bridging/ios/MCShaderFactoryInterface.h (1)

• 31-31: The method signature for createPolygonGroupShader correctly includes the BOOL isStriped parameter, aligning with the PR objectives for iOS bridging code.

ios/graphics/Shader/PolygonGroupShader.swift (2)

• 19-22: The init method correctly takes a isStriped boolean parameter, aligning with the PR objectives for dynamic pipeline selection based on the striped pattern.
• 28-29: The setupProgram method correctly uses the isStriped flag to determine the pipeline type, ensuring dynamic shader configuration based on the striped pattern.

android/src/main/cpp/graphics/shader/ColorPolygonGroup2dShaderOpenGl.h (2)

• 22-22: The constructor now correctly takes a bool isStriped parameter, enabling dynamic shader configuration based on the striped pattern.
• 40-47: The dynamic setting of sizeStyleValues based on the isStriped parameter is a logical approach to handle different shader configurations for striped patterns.

ios/graphics/Shader/ShaderFactory.swift (1)

• 20-21: The createPolygonGroupShader function's signature has been correctly updated to include a _ isStriped: Bool parameter, aligning with the PR objectives for iOS shader factory updates.

shared/public/ShaderFactoryInterface.h (1)

• 35-35: The method signature for createPolygonGroupShader correctly includes the bool isStriped parameter, aligning with the PR objectives for shared shader factory interface updates.

android/src/main/cpp/graphics/objects/PolygonGroup2dOpenGl.h (1)

• 60-60: The addition of scaleFactorHandle as a member variable is noted. Ensure its usage aligns with the intended shader configuration for handling scaling factors in rendering.

Verification successful

---

The verification process confirms that scaleFactorHandle is correctly utilized for shader configuration in both LineGroup2dOpenGl.cpp and PolygonGroup2dOpenGl.cpp. The usage involves obtaining a uniform location with glGetUniformLocation and setting the uniform value with glUniform1f or glUniform2f, which is consistent with OpenGL practices for handling scaling factors in rendering. Therefore, the addition and usage of scaleFactorHandle align with the intended shader configuration.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Verify the usage of scaleFactorHandle in the corresponding .cpp file to ensure it's correctly utilized for shader configuration.
grep -R "scaleFactorHandle" android/src/main/cpp/graphics/objects/

Length of output: 858

android/src/main/cpp/graphics/shader/ShaderFactoryOpenGl.cpp (1)

• 49-50: The createPolygonGroupShader function correctly takes a bool isStriped parameter and initializes a ColorPolygonGroup2dShaderOpenGl object with this parameter, aligning with the PR objectives for dynamic shader configuration.

shared/src/map/layers/tiled/vector/tiles/polygon/Tiled2dMapVectorPolygonTile.h (1)

• 71-71: The addition of the isStriped boolean member variable to the Tiled2dMapVectorPolygonTile class aligns with the PR objectives for supporting striped patterns in polygons.

djinni/graphics/shader/shader.djinni (1)

• 27-27: The create_polygon_group_shader method in the shader_factory_interface interface has been correctly updated to include an is_striped parameter, aligning with the PR objectives for supporting striped patterns in shader creation.

ios/graphics/Model/Polygon/PolygonGroup2d.swift (3)

• 24-24: The addition of posOffset as a private variable for offsetting positions is noted. Ensure its usage is consistent with the intended logic for handling striped patterns.
• 82-87: The handling of the isStriped flag in the render method, including setting scale factors and calculating position offsets based on vertex data, aligns with the PR objectives for dynamic rendering based on the striped pattern.
• 119-135: The logic added to calculate position offsets based on vertex data when isStriped is true is a necessary step for rendering striped patterns accurately.

android/src/main/cpp/graphics/objects/PolygonGroup2dOpenGl.cpp (2)

• 73-73: The addition of scaleFactorHandle to handle scaling factors for rendering is noted. Ensure its usage in the setup method aligns with the intended shader configuration.
• 123-126: The usage of scaleFactorHandle to set uniform values in the shader program for handling scaling factors is correctly implemented, aligning with the PR objectives for dynamic rendering configurations.

bridging/android/jni/graphics/shader/NativeShaderFactoryInterface.h (2)

• 41-41: The createPolygonGroupShader method's signature in the NativeShaderFactoryInterface class has been correctly updated to include a bool isStriped parameter, aligning with the PR objectives for JNI method ID declaration updates.
• 59-59: The JNI method ID declaration for createPolygonGroupShader correctly reflects the updated method signature, ensuring proper JNI integration for the striped pattern feature.

shared/public/PolygonVectorLayerDescription.h (3)

• 24-31: The addition of stripeWidth parameter to PolygonVectorStyle constructor and its inclusion in the constructor's initialization list align with the PR objectives for supporting striped patterns in polygons.
• 81-88: The getStripeWidth method correctly processes the stripeWidth value, including DPI scaling, aligning with the PR objectives for dynamic striped pattern rendering.
• 90-92: The isStripedPotentially method correctly checks for the presence of stripeWidth, enabling conditional logic for striped pattern rendering.

bridging/android/java/io/openmobilemaps/mapscore/shared/graphics/shader/ShaderFactoryInterface.kt (2)

• 21-21: The createPolygonGroupShader function's signature in ShaderFactoryInterface.kt has been correctly modified to include a Boolean parameter isStriped, aligning with the PR objectives for Kotlin interface updates.
• 80-84: The implementation of createPolygonGroupShader now correctly passes the isStriped parameter to the native method native_createPolygonGroupShader, ensuring proper integration for the striped pattern feature.

android/src/main/cpp/graphics/shader/ColorPolygonGroup2dShaderOpenGl.cpp (3)

• 16-18: Ensure the programName initialization incorporates the isStriped flag correctly for unique identification of shader programs.
• 75-128: The conditional shader code for getVertexShader based on isStriped is correctly implemented. Verify the shader code syntax and logic match the intended striped and standard rendering.
• 132-168: The conditional shader code for getFragmentShader based on isStriped is correctly implemented. Ensure the shader logic for striped rendering, especially the calculation for disPx and adjLineWPx, aligns with the rendering requirements.

ios/graphics/Shader/Metal/PolygonGroupShader.metal (4)

• 21-21: The addition of float2 uv to PolygonGroupVertexOut struct is necessary for striped rendering to calculate texture coordinates.
• 30-35: PolygonGroupStripeStyling struct correctly defines the necessary fields for striped rendering, including color, opacity, and stripe information.
• 64-76: The polygonStripedGroupVertexShader function correctly calculates UV coordinates for striped rendering. Ensure the posOffset buffer is correctly used in the calculation.
• 79-94: The polygonGroupStripedFragmentShader function correctly implements the logic for rendering stripes. Verify the calculation for disPx, totalPx, and adjLineWPx aligns with the intended visual output.

shared/src/map/layers/tiled/vector/sublayers/background/Tiled2dMapVectorBackgroundSubLayer.cpp (1)

• 61-61: The addition of the false parameter to createPolygonGroupShader correctly disables striped rendering for this specific layer. Ensure this aligns with the intended behavior for background sublayers.

ios/graphics/Pipelines/PipelineLibrary.swift (5)

• 123-123: The addition of polygonStripedGroupShader to the PipelineType enum correctly extends the pipeline types to include striped rendering.
• 141-141: The label for polygonStripedGroupShader is correctly defined, ensuring clarity in the shader's purpose.
• 161-161: The vertex shader name for polygonStripedGroupShader is correctly specified, aligning with the expected Metal shader function.
• 181-181: The fragment shader name for polygonStripedGroupShader is accurately provided, ensuring the correct shader code is utilized for striped rendering.
• 199-199: The vertex descriptor for polygonStripedGroupShader is correctly set to use PolygonVertex.descriptor, which is necessary for the shader's functionality.

bridging/ios/MCShaderFactoryInterface+Private.mm (2)

• 81-83: The modification to createPolygonGroupShader correctly adds the isStriped parameter, allowing for conditional shader creation based on the striped flag. Ensure the parameter is correctly passed to the C++ method.
• 174-177: The C++ createPolygonGroupShader method signature correctly accepts a boolean parameter c_isStriped, aligning with the Objective-C interface change.

bridging/android/jni/graphics/shader/NativeShaderFactoryInterface.cpp (2)

• 69-74: Ensure the JNI method signature for createPolygonGroupShader correctly handles the new isStriped boolean parameter, following JNI conventions for boolean to jboolean conversion.
• 180-184: The JNI export Java_io_openmobilemaps_mapscore_shared_graphics_shader_ShaderFactoryInterface_00024CppProxy_native_1createPolygonGroupShader correctly translates the jboolean to a C++ bool for isStriped. This is consistent with JNI best practices.

shared/src/map/layers/tiled/vector/tiles/polygon/Tiled2dMapVectorPolygonTile.cpp (3)

• 28-29: Initialization of isStriped in the constructor correctly uses the description->style.isStripedPotentially() method, ensuring the flag is set based on the layer description.
• 110-114: The handling of the isStriped flag in the update method correctly adjusts shader styles to include stripe width if isStriped is true. This logic aligns with the feature's requirements.
• 189-189: The use of isStriped in setVectorTileData to conditionally create a striped or non-striped shader is correct and ensures that the rendering matches the polygon's style description.

shared/src/map/layers/tiled/vector/Tiled2dMapVectorLayerParserHelper.cpp (1)

• 384-385: The addition of stripe-width parsing in parseStyleJsonFromString correctly integrates the new style parameter into the polygon vector style. This change allows for the specification of stripe width in style JSON, aligning with the feature's objectives.