search

geometalab / Vector-Tiles-Reader-QGIS-Plugin

Vector Tiles Reader QGIS-Plugin - QGIS Python plugin which reads Mapbox Vector Tiles from a server, a local MBTiles file or a directory
http://plugins.qgis.org/plugins/vector_tiles_reader/
GNU General Public License v2.0
147 stars 29 forks source link

Inconsistent rendering of custom MB styles #217

Open jwhazel opened 5 years ago

jwhazel commented 5 years ago

OSX 10.14.2 QGIS 3.4.2 Plugin version 2.04

I encounter occasional quirks between the actual custom styles created in MB vs what this plugin renders in qgis.

Example of a custom style (we'll call this v1): https://gist.github.com/jwhazel/e0d5509bf4304b17a7abe3765fba1a61 map1

What actually gets rendered in qgis: map2

Not quite right. So click new and create a new connection... same exact tilejson source, same exact style layer source, call it v2. This is how it gets rendered: map3

If this is a known issue, please lmk. I've searched open and closed issues and didn't quite see anything related. If there is an easy way to debug this, please let me know. I've attempted to close and restart qgis numerous times, clear the tile cache, and reinstall the plugin.

mnboos commented 5 years ago

Thanks for reporting thia issue. Indeed interesting behaviour. Could you show me the logs from both? I guess it's related to QGIS, otherwise it wouldn't be correct the second time.

Somewhere in the back of my head I mean to remember something similar but can't say what it really was.

jwhazel commented 5 years ago

Sure thing. Which logs are you interested in? I know they end up split into several places.

mnboos commented 5 years ago

The Vector Tiles Reader logs, which should be shown somwhere in a log panel.

This issue might be related: https://github.com/geometalab/Vector-Tiles-Reader-QGIS-Plugin/issues/155

jwhazel commented 5 years ago

While I don't see anything in the log panel other than the fact the Vector Tiles Reader was loaded successfully, I do open qgis via terminal which I'm pretty sure outputs just about everything. Here is the entire log from a from the start of opening a fresh copy up till the point where I loaded v2 above. Just to be clear, I'm pretty sure v2 isn't even loading the style layer. https://gist.github.com/jwhazel/86bf43cfc50bc6fb3bc67bcd11c8a4b3

If it would help, I could maybe send you a copy of the my tile layer + style layer url's along with a key.

mnboos commented 5 years ago

Unfortunately, this contains no entries of the VT Reader. However, you can still send me the urls so that I can try it.

jwhazel commented 5 years ago

Tile and style url's sent via email.

jwhazel commented 5 years ago

Sorry to bump but any headway on this?

mnboos commented 5 years ago

Sorry for the delay, I currently quite occupied at the office. I'll try to have a look at it soon.

mnboos commented 5 years ago

Unfortunately, the styles aren't working at all for me. I'm investigating...

mnboos commented 5 years ago

@jwhazel Did you change the style, since you first created this issue? How did you create the style?

I'm currently releasing v2.0.7 which fixes an error in the style generation. Unfortunately, this won't help in this case because your style contains this:

"line-color": ["interpolate", ["exponential", 1], ["zoom"], 3, "#ececec", 4, "#ececec"],

which I currently don't understand.

jwhazel commented 5 years ago

I haven't. I'm not sure how to explain this very well. But basically the style application was intermittent. Since I posted that original photo, I have uninstalled and reinstalled the plugin and now it won't pull any style at all (looks like the last photo I posted). I've noticed this in other styles as well, they were working (albeit inconsistently) and now they are not.

That line that you posted is a camera expression: https://docs.mapbox.com/mapbox-gl-js/style-spec/#camera-expression Not sure if those docs help you to understand it better

mnboos commented 5 years ago

It seems my parser misses many features of the style specification, since I last worked on it. I fixed it now so that it at least should not crash. However, I tested this only based on the style you sent to me by mail. Unfortunately, QGIS crashes now constantly and even though it should not be due to the parser changes, I won't release it as is. I opened a QGIS ticket and hope the developers are able to help / solve the problem.

@sfkeller The problem seems to be the same which caused the unpredictable QGIS crashes since we started working on this plugin. Any ideas what to do?

sfkeller commented 5 years ago

I'll check with QGIS devs. I've seen your QGIS issue here: https://issues.qgis.org/issues/21412 . Can you perhaps describe there a bit more how to reproduce the crash?

mnboos commented 5 years ago

It's difficult to say. The only consistent thing is the "Creating layers..." log entry before the crash. In the code are layers created or reloaded within a loop. I guess it's related to multithreading within QGIS.

tomchadwin commented 5 years ago

Are you explicitly using threads? If so, can you instead use the more recent background tasks API instead?

https://docs.qgis.org/testing/en/docs/pyqgis_developer_cookbook/tasks.html

mnboos commented 5 years ago

No, I don't. I meant more the QGIS internal threading which I noticed by having a look at the logfile of the VT Reader because each log entry also prints his thread.

Thanks for the link anyway, looks interesting. Unfortunately only for QGIS 3.

tomchadwin commented 5 years ago

Aren't you ditching QGIS2 support dev now that it's EOL?

mnboos commented 5 years ago

Hmm I thought about this. Would definitly improve development and maintainability, also due to Python 3 type hints.

@sfkeller Do you know how many people still use QGIS 2?

tomchadwin commented 5 years ago

Because QGIS doesn't dial home on startup (to respect users' privacy), usage stats are not known. I am confident that more people will still be using 2 than 3. But, you can branch off the last v2-compaible version, and move forward with support only for 3. The plugins repo supports multiple versions for multiple QGIS targets.

sfkeller commented 5 years ago

I'd propose the same as @tomchadwin wrote above: Branch off and move forward to support only QGIS 3.

FYI: QGIS 2.18 LTR has IMHO been now superceeded by QGIS 3.4.5 LTR. So if still any issues come in regarding 2 I'd refer and focus to QGIS 2.18 LTR - if at all.

mbernasocchi commented 5 years ago

you can then just change the metadata in the new branch saying min version 3.0 and you'll have two parallel living packages in the repo

sfkeller commented 5 years ago

Coming back to the thread, @mnboos wrote:

It's difficult to say. The only consistent thing is the "Creating layers..." log entry before the crash. In the code are layers created or reloaded within a loop. I guess it's related to multithreading within QGIS.

Can you flesh out the code diff, i.e. the code you changed from current to just before it's crashing now?

mnboos commented 5 years ago

Sounds like a plan, let's do it.

mnboos commented 5 years ago

@mbernasocchi Do I have to change the version for QGIS 2 to max. 2.98, so that only one version targets >= 3?

mnboos commented 5 years ago

@jwhazel Release v3.0.1 has been published. Would you mind giving feedback?

jwhazel commented 5 years ago

@mnboos So far so good! I am seeing some minor differences between the actual Mapbox WebGL rendered version and the version that's being rendered in QGIS but I'm thinking this may have to do with some of the aforementioned mystery specs being ignored.

Screen Shot 2019-03-14 at 1 11 39 PM (official on left, qgis on right)

Not a huge blocker for me since it is at least applying the styles consistently now when creating new connections. So I'm fine with closing this.

I also want to point out that this plugin is now about 10x faster than before when it comes to rendering. I suspect due to the move to 3.x branch. Not part of the original issue, but kudos for this.

mnboos commented 5 years ago

I plan on improving the style converter even though it'll probably be a slow development.

Happy to hear about the performance increase.