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

Highway shield (glyph?) text not centered. #279

Closed
acalcutt opened this issue Apr 27, 2022 · 15 comments
Closed

Highway shield (glyph?) text not centered. #279

acalcutt opened this issue Apr 27, 2022 · 15 comments

Comments

@acalcutt
Copy link
Collaborator

Hi Everyone,

I have been trying to make a maplibre version of Tileserver-GL with the newly revived node version of maplibre-gl-native which is up on github here ( https://github.com/acalcutt/tileserver-gl )

For the most page it all seems to be working, however on the "raster" pages which rely on maplibre-gl-native in the background, I notice the text for highway shields is not getting centered. like this
shields_after

With the mapbox-gl-native used before, it looks correct like this
shields_before

I brought this up on slack and another user said they had the same issue with the shields and had to move to mapbox-gl-native to fix the issue. since someone else confirmed this issues, i figured I would make a post here.

I am using this with this style, which is based on OMT Bright style
https://github.com/acalcutt/wifidb-tileserver-gl/blob/master/tileserver-gl/styles/WDB_OSM/style.json#L4724-L4773

here is a live example
https://tiles.wifidb.net/styles/WDB_OSM/?raster#11/42.2143/-71.8190

@lseelenbinder
Copy link
Member

Thanks for making this ticket @acalcutt. I'm that other user, and in fact I had issues with all label positioning. For example, city labels that should have been top-left aligned where bottom-left aligned.

@acalcutt
Copy link
Collaborator Author

acalcutt commented May 2, 2022

Thanks @lseelenbinder for confirming here.

I spent a little bit of time testing and did confirm just changing back to mapbox-gl-native did make the labels appear normal again. I did some comparing between mapbox 5.0.2 and the current maplibre, but there are many changes since that release so nothing stuck out.

I was mainly looking around symbol layout based on these previous mapbox shield text issues
mapbox/mapbox-gl-native#5683
mapbox/mapbox-gl-native#9395

@acalcutt
Copy link
Collaborator Author

acalcutt commented May 9, 2022

I've uploaded a test style and a mbtiles file of massachusetts which includes shields at ( https://github.com/acalcutt/tileserver-gl/releases/download/test_data/test_data_massachusetts.zip )

This is an example of using the test data in the docker version of acalcutt\tileserver-gl (Ubuntu,Debian,WSL)
1.) git clone https://github.com/acalcutt/tileserver-gl.git
2.) cd tileserver-gl
4.) wget https://github.com/acalcutt/tileserver-gl/releases/download/test_data/test_data_massachusetts.zip
5.) unzip test_data_massachusetts.zip
3.) docker build --no-cache -t test1 .
6.) docker run -v $(pwd):/data -p 8080:80 test1
7.) Browse to 127.0.0.1:8080 and click 'raster' under the WDB_OSM style and zoom in to massachusetts (eg. http://127.0.0.1:8080/styles/wdb-osm/?raster#10/42.2595/-71.9028 ) .
8.) see shields and text not looking correct
9.) Ctrl+C in the docker window to kill tileserver-gl

@rmyhal
Copy link
Contributor

rmyhal commented May 16, 2022

@acalcutt the issue was introduced in this PR
I was going through different commits and found that on this commit the issue appeared.

cc @ntadej , could you please help us?

@acalcutt
Copy link
Collaborator Author

acalcutt commented May 16, 2022

I can confirm, I tested reverting #270 in @acalcutt/[email protected] and manually changed my local tileserver-gl to use that release and it seems like it worked

image

@acalcutt
Copy link
Collaborator Author

acalcutt commented May 17, 2022

While reverting #270 does fix the shields, there is still a bit of something going on with the text, for example.

With
docker run --rm -it -v $(pwd):/data -p 8080:80 wifidb/tileserver-gl
maplibre

With
docker run --rm -it -v $(pwd):/data -p 8080:80 maptiler/tileserver-gl
mapbox

This should probably be a separate issue though.

@ntadej
Copy link
Collaborator

ntadej commented May 17, 2022

Is this node bindings only? As I see no issues with text in apps.

Will look into shields.

@rmyhal
Copy link
Contributor

rmyhal commented May 17, 2022

Is this node bindings only? As I see no issues with text in apps.

Will look into shields.

I found the problem. Here you made a typo during adding static_cast and misplaced brackets. The fix looks like this:
shiftY = (-verticalAlign * static_cast<float>(lineCount) + 0.5f) * lineHeight;

@lseelenbinder
Copy link
Member

@acalcutt The cut text issue is likely unrelated; that looks like typical label clipping issues that are fairly common with the NodeJS bindings. They don’t always properly set the map mode settings for tiles (vs larger area rendering).

@acalcutt
Copy link
Collaborator Author

acalcutt commented May 17, 2022

I had seen this when researching the shield issue mapbox/mapbox-gl-native#15880 which at least sounds similar to the label issue i am seeing. though when i looked at the maplibre code it looked like some of that fix made it over already.

Not sure if this is a node issue since it didnt happen (or at least didn't happen as much) with the mapbox version (as you can see from the screenshot of the maptiler version

acalcutt added a commit to acalcutt/maplibre-gl-native that referenced this issue May 17, 2022
acalcutt added a commit to acalcutt/tileserver-gl that referenced this issue May 17, 2022
@acalcutt
Copy link
Collaborator Author

I can confirm the line rmyhal mentioned seems to work. I put back #270 and applied that line fix and the shields look correct.

@acalcutt
Copy link
Collaborator Author

I could put in a PR with that fix, unless @rmyhal wants to put it in.

@rmyhal
Copy link
Contributor

rmyhal commented May 18, 2022

thanks @acalcutt !
I can fill a PR soon

@rmyhal
Copy link
Contributor

rmyhal commented May 19, 2022

@acalcutt

#285

@ntadej
Copy link
Collaborator

ntadej commented May 20, 2022

Closing this one. Please reopen if there are any further issues.

@ntadej ntadej closed this as completed May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants