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.

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

Update style by clearing cache #2130

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

a-dutremble
Copy link
Contributor

@a-dutremble a-dutremble commented Jul 4, 2023

Description

Add some features to update the style of a layer.

We've added a function for redrawing a layer. It selects the layer, invalidates the cache and notifies changes.

With this function, you can change the style of the entire layer or only a specific feature in your layer.
However, it clears and updates all the layer even if you change only one feature in this layer.

We've also added an example to illustrate the function.

PS: The refresh one tile that were already refreshing before the style changement doesn't seem to refresh to the new style

@a-dutremble a-dutremble force-pushed the update_style branch 7 times, most recently from a9180f8 to cb66b44 Compare July 5, 2023 15:00
@a-dutremble a-dutremble marked this pull request as ready for review July 5, 2023 15:10
@a-dutremble a-dutremble changed the title Update style with clearing cache Update style by clearing cache Jul 5, 2023
@a-dutremble a-dutremble force-pushed the update_style branch 5 times, most recently from 6a32960 to 2614786 Compare November 21, 2023 14:07
@jailln jailln self-requested a review December 11, 2023 08:49
@jailln jailln self-assigned this Dec 11, 2023
@ftoromanoff ftoromanoff force-pushed the update_style branch 2 times, most recently from 30e06d5 to d7f8774 Compare January 5, 2024 14:53
@jailln
Copy link
Contributor

jailln commented Oct 2, 2024

Before going into code details, some general remarks:

  • There is a blinking effect when zooming out after changing the style (if the tiles where already loaded):
issue-style.mp4

We should fix that.

  • I'm not sure we should expose a method on the view. I think a method on the layer is enough since it is only related to the layer and not the view. Then, it should be left to the user to call view.notifyChange as it is the current philosophy of itowns.

  • I would rather have a Layer.updateStyle(newStyle) method to make it more explicit (or use the style property setter?). A bit like what has been implemented for C3DTilesLayer. This method should only reapply the style conversion to this layer. Without looking into too much yet (so there might be incompatibilities in doing this), this could mean:

    • taking the styling step out of convert - into a new style step for instance (this may have more sense for Feature2Mesh than for Feature2Texture)
    • reapply this step to the layer when we call updateStyle() (or the style setter)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants