Repository: brendan-ward/pymgl (language: python)

Issue:
ENH: Force remote styles to load sooner
This forces remote styles to load as as soon as the `Map` object is instantiated rather than waiting for the first render call.  This ensures that sources and layers are populated from the remote style before accessing them.

This also adds a log observer to try and detect parsing errors that are raised internally in maplibre-gl-native as warnings.


Discussion / hints:

Fix patch:
diff --git a/README.md b/README.md
index 5f5628c..fcd0eef 100644
--- a/README.md
+++ b/README.md
@@ -188,7 +188,34 @@ map.listLayers()  # [<layerId1>, ...]
 
 map.listSources()  # [<sourceId1>, ...]
 
-map.getLayerJSON()  # returns JSON describing layer
+map.getLayerJSON(<layerId>)  # returns JSON describing layer
+```
+
+IMPORTANT: if you are using a remotely-hosted style, you need to force the map
+to render - which loads all underying assets - before listing the style's layers,
+sources, or other properties.
+
+```Python
+map = Map("mapbox://styles/mapbox/streets-v11", token=<mapbox token>, provider="mapbox")
+
+map.listLayers()  # []
+map.renderPNG()
+map.listLayers()  # [<layerId1>, ...]
+```
+
+Alternatively, you can download the style yourself and provide that as input to
+the Map, and it will show all layers without requiring a render first. However, not all assets will be loaded until the first render.
+
+```Python
+from urllib.request import urlopen
+
+url = f"https://api.mapbox.com/styles/v1/mapbox/streets-v11?access_token={MAPBOX_TOKEN}"
+
+with urlopen(url) as r:
+    style = r.read()
+
+map = Map(style.decode("UTF-8") token=<mapbox token>, provider="mapbox")
+map.listLayers()  # [<layerId1>, ...]
 ```
 
 You can auto-fit the map to bounds instead of using center longitude / lantitude
@@ -464,10 +491,6 @@ cd vendor/nanobind
 git submodule update --init --recursive
 ```
 
-Temporary fix: update `vendor/nanobind/nanobind-config.cmake` to comment out
-`COMMAND xcodebuild -version` and
-`COMMAND pkgutil --pkg-info=com.apple.pkg.CLTools_Executables | awk '/version:/ {print $2}'`
-
 ### Maplibre-gl-native
 
 Maplibre-gl-native is included as a git submodule, and it includes many submodules
diff --git a/docker/README.md b/docker/README.md
index 0c0e147..a4e474e 100644
--- a/docker/README.md
+++ b/docker/README.md
@@ -23,9 +23,6 @@ python3 setup.py develop
 # tests require virtual display
 Xvfb ${DISPLAY} -screen 0 "1024x768x24" -ac +render -noreset -nolisten tcp  &
 
-# or this if using GLX:
-# Xvfb ${DISPLAY} -screen 0 "1024x768x24" -ac +extension GLX +render -noreset  -nolisten tcp  &
-
 export MAPBOX_TOKEN=<your token>
 
 # run C++ test suite
diff --git a/mbgl-core/maplibre-gl-native-linux.cmake b/mbgl-core/maplibre-gl-native-linux.cmake
index cf06fdb..6b24d05 100644
--- a/mbgl-core/maplibre-gl-native-linux.cmake
+++ b/mbgl-core/maplibre-gl-native-linux.cmake
@@ -13,32 +13,30 @@ find_package(X11 REQUIRED)
 # NOTE: EGL is optional and backfilled by GLX in mapblibre-gl-native, but
 # we require installing EGL to simplify build
 find_package(OpenGL REQUIRED EGL)
-# find_package(OpenGL REQUIRED GLX)
 
 pkg_search_module(LIBUV libuv REQUIRED)
 
 target_sources(
     mbgl-core
     PRIVATE
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/storage/http_file_source.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/i18n/collator.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/i18n/number_format.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/text/local_glyph_rasterizer.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/async_task.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/compression.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/image.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/jpeg_reader.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/logging_stderr.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/png_reader.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/png_writer.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/run_loop.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/string_stdlib.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/thread.cpp
-        ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/timer.cpp
-        ${MBGL_SOURCE_DIR}/platform/linux/src/gl_functions.cpp
-
-        ${MBGL_SOURCE_DIR}/platform/linux/src/headless_backend_egl.cpp
-        # ${MBGL_SOURCE_DIR}/platform/linux/src/headless_backend_glx.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/storage/http_file_source.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/i18n/collator.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/i18n/number_format.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/text/local_glyph_rasterizer.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/async_task.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/compression.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/image.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/jpeg_reader.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/logging_stderr.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/png_reader.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/png_writer.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/run_loop.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/string_stdlib.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/thread.cpp
+    ${MBGL_SOURCE_DIR}/platform/default/src/mbgl/util/timer.cpp
+    ${MBGL_SOURCE_DIR}/platform/linux/src/gl_functions.cpp
+
+    ${MBGL_SOURCE_DIR}/platform/linux/src/headless_backend_egl.cpp
 )
 
 include(${MBGL_SOURCE_DIR}/vendor/nunicode.cmake)
@@ -47,15 +45,14 @@ include(${MBGL_SOURCE_DIR}/vendor/sqlite.cmake)
 target_link_libraries(
     mbgl-core
     PRIVATE
-        ${CURL_LIBRARIES}
-        ${JPEG_LIBRARIES}
-        ${LIBUV_LIBRARIES}
-        ${X11_LIBRARIES}
-        ICU::i18n
-        ICU::uc
-        PNG::PNG
-        mbgl-vendor-nunicode
-        mbgl-vendor-sqlite
-        OpenGL::EGL
-        # OpenGL::GLX
+    ${CURL_LIBRARIES}
+    ${JPEG_LIBRARIES}
+    ${LIBUV_LIBRARIES}
+    ${X11_LIBRARIES}
+    ICU::i18n
+    ICU::uc
+    PNG::PNG
+    mbgl-vendor-nunicode
+    mbgl-vendor-sqlite
+    OpenGL::EGL
 )
diff --git a/src/map.cpp b/src/map.cpp
index 591f279..d55ddec 100644
--- a/src/map.cpp
+++ b/src/map.cpp
@@ -136,19 +136,8 @@ Map::Map(const std::string &style,
         map->getStyle().loadJSON(style);
     } else if (style.find("://") != -1) {
         // otherwise must be URL-like reference, like "mapbox://styles/mapbox/streets-v11"
-
+        // if local, must be an absolute path: file://<absolute_path>
         map->getStyle().loadURL(style);
-
-        // run the looop until the style is loaded
-        observer->didFinishLoadingStyleCallback = [&]() { loop->stop(); };
-
-        observer->didFailLoadingMapCallback
-            = [&](mbgl::MapLoadError type, const std::string &description) {
-                  loop->stop();
-                  throw std::runtime_error(description);
-              };
-
-        loop->run();
     } else if (style.empty()) {
         // construct blank JSON
         map->getStyle().loadJSON(R"({


Test patch:
diff --git a/pymgl/tests/test_map.py b/pymgl/tests/test_map.py
index 2b9818a..6cc9106 100644
--- a/pymgl/tests/test_map.py
+++ b/pymgl/tests/test_map.py
@@ -201,6 +201,9 @@ def test_list_layers_remote_style():
         provider="mapbox",
     )
 
+    # remote styles require render to load all assets
+    map.renderPNG()
+
     assert len(map.listLayers()) == 111
 
 
@@ -225,6 +228,9 @@ def test_list_sources_remote_style():
         provider="mapbox",
     )
 
+    # remote styles require render to load all assets
+    map.renderPNG()
+
     assert map.listSources() == ["composite"]
 
 
diff --git a/pymgl/tests/test_style.py b/pymgl/tests/test_style.py
index 711311b..388256a 100644
--- a/pymgl/tests/test_style.py
+++ b/pymgl/tests/test_style.py
@@ -91,8 +91,8 @@ def test_mapbox_style_missing_token():
 
 
 def test_bad_remote_style_url():
-    with pytest.raises(RuntimeError, match="loading style failed"):
-        Map("https://google.com/bogus_style")
+    with pytest.raises(RuntimeError, match="HTTP status code 404"):
+        Map("https://google.com/bogus_style").renderPNG()
 
 
 @pytest.mark.skipif(not has_poorconn, reason="poorconn test lib not available")
diff --git a/tests/StyleTest.cpp b/tests/StyleTest.cpp
index 5d1ab85..b266007 100644
--- a/tests/StyleTest.cpp
+++ b/tests/StyleTest.cpp
@@ -67,6 +67,39 @@ TEST(Style, FileGeoJSON) {
     EXPECT_TRUE(image_matches(img_filename, 10));
 }
 
+TEST(Style, LocalStyleGeoJSON) {
+    const string test = "example-style-geojson";
+    string style      = "file://" + FIXTURES_PATH + test + ".json";
+
+    Map map = Map(style, 100, 100, 1);
+    map.setBounds(-125, 37.5, -115, 42.5);
+    auto img = map.renderPNG();
+
+    const string img_filename = test + ".png";
+
+    // to write out expected image, uncomment
+    // write_test_image(img, img_filename, true);
+
+    write_test_image(img, img_filename, false);
+    EXPECT_TRUE(image_matches(img_filename, 10));
+}
+
+TEST(Style, LocalStyleFileRemoteRaster) {
+    const string test = "example-style-remote-raster";
+    string style      = "file://" + FIXTURES_PATH + test + ".json";
+
+    Map map  = Map(style, 256, 256, 1);
+    auto img = map.renderPNG();
+
+    const string img_filename = test + ".png";
+
+    // to write out expected image, uncomment
+    // write_test_image(img, img_filename, true);
+
+    write_test_image(img, img_filename, false);
+    EXPECT_TRUE(image_matches(img_filename, 10));
+}
+
 TEST(Style, RemoteRaster) {
     const string test  = "example-style-remote-raster";
     const string style = read_style(test + ".json");
@@ -127,8 +160,10 @@ TEST(Style, MapboxStyle) {
         GTEST_SKIP_("Missing Mapbox Token");
     }
 
-    Map map  = Map("mapbox://styles/mapbox/streets-v11", 256, 256, 1, 0, 0, 0, token, "mapbox");
+    Map map = Map("mapbox://styles/mapbox/streets-v11", 256, 256, 1, 0, 0, 0, token, "mapbox");
+    std::cout << "Before render map" << std::endl;
     auto img = map.renderPNG();
+    std::cout << "After render map" << std::endl;
 
     const string img_filename = "mapbox-streets-v11.png";
 
@@ -140,7 +175,9 @@ TEST(Style, MapboxStyle) {
 }
 
 TEST(Style, BadRemoteStyleURL) {
-    EXPECT_THROW(Map("https://google.com/bogus_style", 10, 10), std::runtime_error);
+    Map map = Map("https://google.com/bogus_style", 10, 10);
+
+    EXPECT_THROW(map.renderPNG(), std::runtime_error);
 }
 
 TEST(Style, Labels) {
diff --git a/tests/WrapperTest.cpp b/tests/WrapperTest.cpp
index 7ae87df..c9e873a 100644
--- a/tests/WrapperTest.cpp
+++ b/tests/WrapperTest.cpp
@@ -309,6 +309,9 @@ TEST(Wrapper, ListLayersRemoteStyle) {
     }
 
     Map map = Map("mapbox://styles/mapbox/streets-v11", 10, 10, 1, 0, 0, 0, token, "mapbox");
+    // remote map requires render call to load all remote assets
+    map.renderPNG();
+
     EXPECT_EQ(map.listLayers().size(), 111);
 }
 
@@ -330,7 +333,10 @@ TEST(Wrapper, ListSourcesRemoteStyle) {
         GTEST_SKIP_("Missing Mapbox Token");
     }
 
-    Map map      = Map("mapbox://styles/mapbox/streets-v11", 10, 10, 1, 0, 0, 0, token, "mapbox");
+    Map map = Map("mapbox://styles/mapbox/streets-v11", 10, 10, 1, 0, 0, 0, token, "mapbox");
+    // remote map requires render call to load all remote assets
+    map.renderPNG();
+
     auto sources = map.listSources();
     EXPECT_EQ(sources.size(), 1);
     EXPECT_EQ(sources[0], "composite");