Dynamic style update for feature meshes#2648
Conversation
Neptilo
force-pushed
the
dynamic-style-update
branch
from
aa1cd66 to
f0d148e
Compare
|
Thanks for the PR ! Would it be possible to add unit tests for the new behavior that you add here ? (Mostly for Feature2Mesh.js) And maybe an example where we can play with this ? |
Neptilo
force-pushed
the
dynamic-style-update
branch
from
bf18706 to
3feb58c
Compare
|
@ftoromanoff @mgermerie PR ready again |
|
ping @ftoromanoff @mgermerie |
|
I don't know if it's linked with that PR, but when I play with the Layer WFS Building' opacity and that I tilt the view I get some strange results: |
mgermerie
left a comment
mgermerie
left a comment
There was a problem hiding this comment.
Thanks for the PR, and nice idea for the style properties versioning implementation!
A few comments on the code other than which there is an issue with line altitude: it seems you have an accidental increment somewhere, making each line appear higher than the previous one. You can see this behavior on this screenshot:
packages/Main/src/Core/Style.js
Outdated
| getGeometry() { | ||
| return this.#geometry; | ||
| } | ||
|
|
There was a problem hiding this comment.
Maybe a proper get geometry would be more alike what's done with properties getter. I know setters are not defined as so but we can try at least to have coherence in our getters.
| * @param {number} buffers.indexPtr - Current write position in index buffer (incremented by function). | ||
| * @param {number} [id] - Batch id value to assign to written vertices when batchIds is provided. | ||
| */ | ||
| function updateLineBuffers(featureMesh, buffers, id) { |
There was a problem hiding this comment.
For more code navigability and consistency with your other updates, can you either define this method right after featureToLine or define all other update{shape}Buffer methods here along side this one ?
| up.set(0, 0, 1).multiply(inverseScale); | ||
|
|
||
| const pointMaterialSize = []; | ||
| context.setFeature(feature); |
There was a problem hiding this comment.
You re-call this inside the updatePointBuffer method. I get the need of it inside updatePointBuffer method (which can be called by updateStyle) but it thus should be removed from here. Same goes for context.setGeometry on line 205 and in other feature type methods.
There was a problem hiding this comment.
Good catch. For context.setGeometry, I don't see the redundancy. All the update*Buffers seem to access context.geometry and therefore need the geometry to be defined before.
From my personal testing, I did not have this issue, buildings opacity was correctly computed. I tried on the WFS to 3D objects example, with custom or identical color for each buildings, before and after toggling their extrusion height. Maybe you found this in another test condition @ftoromanoff ? |
Found it! I had oversimplified by getting |
I can´t reproduce either. In the image it looks like the terrain also has transparency. |
4 participants
Addresses #2649
Description
Enables dynamic style updates for feature meshes. Refactors buffer logic into reusable functions and adds style versioning to track property changes.
Motivation and Context
Allows real-time style updates (colors, heights) without rebuilding meshes, improving performance for interactive applications.