maplibre-gl-js icon indicating copy to clipboard operation
maplibre-gl-js copied to clipboard

Symbol texts shouldn't rendering above all images on same layer

Open gitcatrat opened this issue 4 years ago • 13 comments

Motivation

See this issue for context.

I think this should be a priority(ish) for both Mapbox and this fork (issue has been open for 4 years). One of the main reasons to render markers and clusters with WebGL is performance and stuff like generating icon textures on the fly defeats that purpose (which is feasible if you have couple hundred different image-text combinations in the first place).

I think this is quite common use case and I'm very surprised it hasn't been solved yet.

Design Alternatives

AFAIK, issue exists because symbol images are rendered in one pass and texts in another which causes all texts rendered on top of all images on same layer.

I'm not sure what's the great solution to this. ..

  • Render symbols as a single unit (i.e both image and text)?
  • Track relationships and set text "z-index" value to match with related image?

Rendering performance is probably going to get worse (i.e time per 1000 symbols or something) if both images and texts are used in same symbol but it's still a lot better than broken implementation.

Design

Let's discuss.

Mock-Up

Concepts

Implementation

gitcatrat avatar Dec 28 '20 23:12 gitcatrat

I believe my pull request https://github.com/mapbox/mapbox-gl-js/pull/10123 fixes this issue. I was planing to redo the pull request for maplibre anyway when the fork has settled enough to develop new features. I this already the case?

Be aware as said in the original pull requests that I do not know the internals or the involved technologies enough to be sure that the approach in the pull request is feasible/correct. I intended to refer to the feedback of more knowledgeable people on that.

xabbu42 avatar Jan 28 '21 14:01 xabbu42

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Nov 02 '21 01:11 github-actions[bot]

This is still on my radar.

xabbu42 avatar Nov 02 '21 08:11 xabbu42

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] avatar May 02 '22 02:05 github-actions[bot]

This should remain open even if a fix is hard. I've also not given up on my fix yet but need more time to make sure it is fast enough. I will stop fighting the stale bot after this though.

xabbu42 avatar May 04 '22 08:05 xabbu42

any news?

PhungVanHoa avatar Oct 31 '22 07:10 PhungVanHoa

The following PR were suggested to fix this but didn't end up in main branch due to different reasons (you can read it in the relevant PR) #145 #985. Feel free to continue pursuing this.

HarelM avatar Oct 31 '22 07:10 HarelM

Unfortunately I could not make time to pursue this further myself. We will need a fix for this though and I will try again in the future. Any help in the meantime is very welcome :-).

xabbu42 avatar Oct 31 '22 10:10 xabbu42

This problem is not fixed yet. Is there any plan for this?

PhungVanHoa avatar Mar 19 '23 10:03 PhungVanHoa

You are welcome to pursue this issue. There is a performance hit related to the original PR and there is a workaround mentioned in the last PR.

HarelM avatar Mar 19 '23 16:03 HarelM

Any updates on the issue? I suffer from the same problem.

Edit: Worked around it by generating the images with a canvas. I optimized this a bit by first getting an instance of ImageData and then placing that on the canvas on each iteration instead of drawing the image on each iteration. does my 500 iterations in 150ms, and 50 in ~7ms. Remember to add "willReadFrequently", else your time will be spent sending and receiving buffers from the gpu.

const canvas = document.createElement('canvas');
const context = canvas.getContext('2d', { willReadFrequently: true })!;
const genAndLoadImages = ([name, url, color]) => loadSvg(url).then(img => {
    context.clearRect(0, 0, img.width, img.height);
    context.drawImage(img, 0, 0);
    const imageWithoutNumber = context.getImageData(0, 0, img.width, img.height);
    canvas.width = img.width;
    canvas.height = img.height;
    context.font = 'bold 44px sans-serif';
    context.textAlign = 'center';
    context.fillStyle = color;
    for (let i = 0; i < 50; i++) {
    context.putImageData(imageWithoutNumber, 0, 0);
    context.fillText(i.toString(), img.width / 2, img.height / 1.6);
    const newImg = context.getImageData(0, 0, img.width, img.height);
    map.addImage(`${name}-${i}`, newImg);
    }
})
function loadSvg(url: string): Promise<HTMLImageElement> {
  return new Promise((resolve, reject) => {
    let img = new Image;
    img.onload = _ => resolve(img);
    img.onerror = reject;
    img.src = url;
  });
}		

Wanted to add this for whoever stumbled on this issue like I did. If it was useful please add a reaction, I'm curious how many people this helped.

Happy mapping!

ttmx avatar Dec 18 '23 11:12 ttmx

Thanks for writing this up! Nice workaround!

HarelM avatar Dec 18 '23 17:12 HarelM