-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
Bug/change image on hover #74
Conversation
work in progress, shows problems with changing images on hover
Ok, so I found the culprit with changes to image symbols and tried to fix it. As far as I understand the code, on changes to features, the corresponding renderprimitives are being removed and in case of images their images (and potentially the used image texture) are also. However, images within I added the same strategy as with the image textures This fixes the problems with disappearing or wrongly changed images on hover (see the example). However, I still have to reuse vacant slots to avoid indefinitely growing list of images... |
ok, vacant image slots are now reused. Let me know if I should change the implementation, but I think this should work and not introduce a lot of overhead (I might be wrong though!) |
Hi, @lennart . Thank you for this work. At the first glance this solution looks fine. I'll look at it in depth later today or tomorrow and then merge it. |
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.
The current approach is fine and works correctly. But I wonder if we could use a data structure that would encapsulate keeping track of vacant ids. The data structure can be generic over what exactly it stores, so it can be used for both image infos and image store.
TBH, I am pretty sure there must be a crate published that does exactly this, as this seems like something useful for many use cases. But I have no idea how to find such a crate without even knowing how such a data structure would be called... Something like StableVec
or something?..
good guess, I'd say, there is stable-vec which does what we currently implement manually, although the crate docs also refer to the more popular slab crate (by tokio-rs) which might be a more reasonable choice (although I haven't yet used any of them nor read the docs in-depth). So I would lean towards using slab (haven't checked the deps of it yet). |
also, do you think this pr should already introduce the external crate, or should that be an additional PR? in case of the latter I'd first fix the parts you commented on, to get this merged. |
add readme entry for new example
This PR should eventually fix choosing different images of symbols depending on the state of the underlying feature.
I added a simple mapping struct
ImageStoreInfo
analogous toPrimitiveInfo
to allow retaining indices of stored images and keep a consistent mapping between images in use and the corresponding image in store. Current main branch seems to have issues with mapping between the images in store and the images as used by symbols as images are removed from the store regardless of whether other symbols are still using them leading to indices into the image store being wrong (i.e. suddenly all marker render a different image instead of just one).To demonstrate the bug and a use-case, I added an example showing highlighting of features (map pins) on mouse hover.
The implementation of the example currently iterates through all layer features and resets their state and afterwards only sets the state for those in proximity of mouse; this seems inefficient, but I did not know how to do it differently.
PR is still draft, as even with the fixes I applied to image store, hovering a pin sometimes completely removes another, or even toggles the wrong feature/image. If you have any suggestions on how to fix the issue, let me know.
You can see the original behavior on
main
by simply checking out both the example and the green-pin.png from this pr's branch and run the example normally (you should see all pins highlighted when hovering just one)