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

[Bug] With hardware acceleration set to false weird behaviour with initial paint for layers #4647

Open
ToHold opened this issue Sep 3, 2024 · 8 comments
Labels
need more info Further information is requested

Comments

@ToHold
Copy link
Contributor

ToHold commented Sep 3, 2024

maplibre-gl-js version: 4.5.0

browser: Chromium 128.0.6613.84

Steps to Trigger Behavior

  1. Disable hardware acceleration
  2. Use style with a fill extrusion layer
  3. Add a layer without paint before this one
  4. Set second layer paint with map.getLayer('layer').setPaintProperty()

Link to Demonstration

https://codepen.io/ToHold/pen/abgRaye

Expected Behavior

This should work like with hardware acceleration on

Actual Behavior

Layer are weirdly overlapping each other. In on of my project with threejs, I can see other app windows behind my browser (like vscode).

@HarelM
Copy link
Collaborator

HarelM commented Sep 3, 2024

Thanks for taking the time to report this issue.
I'm not sure the assumptions that this should work without hardware acceleration is a valid assumption.
Maplibre uses webgl and the GPU heavily...

@HarelM HarelM added the need more info Further information is requested label Sep 3, 2024
@neodescis
Copy link
Collaborator

neodescis commented Sep 3, 2024

We have users running MapLibre without hardware acceleration. It's actually pretty decent if you don't have terrain turned on.

That said, the codepen above has layers overlapping in weird ways for me even with acceleration turned on. The behavior is the same for me with acceleration turned off, aside from being very slow.

@ToHold
Copy link
Contributor Author

ToHold commented Sep 4, 2024

Yes my bad with hardware acceleration, it also happen, should I rename the issue ?

@HarelM
Copy link
Collaborator

HarelM commented Sep 4, 2024

I believe the correct way to do it would be to use the map API and not the layer API:

map.setPaintProperty('rectangle-layer', 'fill-color', [
    'get',
    'fill-color'
  ]);
	
  map.setPaintProperty('rectangle-layer', 'fill-opacity', [
    'get',
    'fill-opacity'
  ]);

The return type of getLayer is StyleLayer and I don't think it support this method.
See here: https://maplibre.org/maplibre-gl-js/docs/API/classes/StyleLayer/

I would strongly advise to use typescript in order to avoid these kind of issues...

@ToHold
Copy link
Contributor Author

ToHold commented Sep 4, 2024

Well I use Typescript, why not prefix it by underscored then ? Maybe @internal tag would be interesting too.

@ToHold
Copy link
Contributor Author

ToHold commented Sep 4, 2024

And yes I also have users without hardware acceleration that's why I encounter some bugs, yes this one isnt related to hardware acceleration but I can't easly reproduce it in a codepen.

@ToHold
Copy link
Contributor Author

ToHold commented Sep 4, 2024

I believe the correct way to do it would be to use the map API and not the layer API:

map.setPaintProperty('rectangle-layer', 'fill-color', [
    'get',
    'fill-color'
  ]);
	
  map.setPaintProperty('rectangle-layer', 'fill-opacity', [
    'get',
    'fill-opacity'
  ]);

The return type of getLayer is StyleLayer and I don't think it support this method. See here: https://maplibre.org/maplibre-gl-js/docs/API/classes/StyleLayer/

I would strongly advise to use typescript in order to avoid these kind of issues...

Well I'll give a try this way, thank you

@HarelM
Copy link
Collaborator

HarelM commented Sep 4, 2024

I've looked at the code, and it is similar but not identical.
I'm not sure setting it to "internal" will actually solve the issue since you'll still be able to call this method.
Here's the relevant code inside style object:

setPaintProperty(layerId: string, name: string, value: any, options: StyleSetterOptions = {}) {
this._checkLoaded();
const layer = this.getLayer(layerId);
if (!layer) {
this.fire(new ErrorEvent(new Error(`Cannot style non-existing layer "${layerId}".`)));
return;
}
if (deepEqual(layer.getPaintProperty(name), value)) return;
const requiresRelayout = layer.setPaintProperty(name, value, options);
if (requiresRelayout) {
this._updateLayer(layer);
}
this._changed = true;
this._updatedPaintProps[layerId] = true;
// reset serialization field, to be populated only when needed
this._serializedLayers = null;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need more info Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants