Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .mise/bin/sync-submodules
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ mlt_vendor_paths=(
mln_patches=(
"$repo_root/patches/maplibre-native/0002-windows-local-file-urls.patch"
"$repo_root/patches/maplibre-native/0003-run-loop-process-gate.patch"
"$repo_root/patches/maplibre-native/0004-opengl-valid-api-calls.patch"
)

# A patch counts as applied when it reverses cleanly, which is also what makes
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package org.maplibre.nativeffi.render

import kotlin.test.Test
import kotlin.test.assertContentEquals
import kotlin.test.assertTrue
import org.maplibre.nativeffi.map.MapHandle
import org.maplibre.nativeffi.map.MapMode
import org.maplibre.nativeffi.runtime.RuntimeEventType
import org.maplibre.nativeffi.runtime.RuntimeHandle

class GoldfishStyleReloadTest {
@Test
fun repeatedSnapshotStyleReloadRendersComposedLayer() {
withOwnedTextureSession(
width = SNAPSHOT_SIZE,
height = SNAPSHOT_SIZE,
mapWidth = SNAPSHOT_SIZE,
mapHeight = SNAPSHOT_SIZE,
mapMode = MapMode.STATIC,
) { runtime, map, owned ->
loadBaseStyle(runtime, map, BASE_STYLE)
addComposition(map)
assertContentEquals(GREEN, captureCenterPixel(runtime, map, owned.session))

loadBaseStyle(runtime, map, ALTERNATE_STYLE)
loadBaseStyle(runtime, map, BASE_STYLE.copyOf())
addComposition(map)

assertContentEquals(GREEN, captureCenterPixel(runtime, map, owned.session))
}
}

private fun loadBaseStyle(runtime: RuntimeHandle, map: MapHandle, style: ByteArray) {
map.setStyleJson(style)
assertTrue(waitForMapEvent(runtime, map, RuntimeEventType.MAP_STYLE_LOADED))
}

private fun addComposition(map: MapHandle) {
map.addStyleSourceJson(COMPOSED_SOURCE_ID, COMPOSED_SOURCE)
map.addStyleLayerJson(COMPOSED_LAYER, "")
}

private fun captureCenterPixel(
runtime: RuntimeHandle,
map: MapHandle,
session: RenderSessionHandle,
): ByteArray {
map.requestStillImage()
var captured: ByteArray? = null
repeat(10_000) {
runtime.pump(0)
var finished = false
for (event in runtime.drainEvents().events.filter { it.mapSource == map }) {
when (event.type) {
RuntimeEventType.MAP_RENDER_UPDATE_AVAILABLE ->
if (session.renderUpdate().result == RenderResult.RENDERED) {
val info = session.textureImageInfo()
NativeBuffer.allocate(info.byteLength).use { buffer ->
session.readPremultipliedRgba8(buffer)
val centerOffset = SNAPSHOT_SIZE / 2 * info.stride + SNAPSHOT_SIZE / 2 * 4
captured = buffer.toByteArray().copyOfRange(centerOffset, centerOffset + 4)
}
}
RuntimeEventType.MAP_STILL_IMAGE_FINISHED -> finished = true
RuntimeEventType.MAP_STILL_IMAGE_FAILED -> error(event.message)
}
}
if (finished) return captured ?: error("still image finished without a rendered frame")
runtime.pump(1)
}
error("still image did not finish")
}

private companion object {
private const val SNAPSHOT_SIZE = 64
private const val COMPOSED_SOURCE_ID = "composed-point"
private val GREEN = byteArrayOf(0, -1, 0, -1)
private val BASE_STYLE =
jsonBytes(
"""{"version":8,"sources":{},"layers":[{"id":"base","type":"background","paint":{"background-color":"#000000"}}]}"""
)
private val ALTERNATE_STYLE =
jsonBytes(
"""{"version":8,"sources":{},"layers":[{"id":"alternate","type":"background","paint":{"background-color":"#0000ff"}}]}"""
)
private val COMPOSED_SOURCE =
jsonBytes("""{"type":"geojson","data":{"type":"Point","coordinates":[0,0]}}""")
private val COMPOSED_LAYER =
jsonBytes(
"""{"id":"composed-circle","type":"circle","source":"composed-point","paint":{"circle-color":"#00ff00","circle-radius":20}}"""
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,11 @@ class ResourceRequestHandleAndroidTest {
}

private fun awaitRelease(released: CountDownLatch): Boolean {
val runtime = Runtime.getRuntime()
repeat(ATTEMPTS) {
runtime.gc()
runtime.runFinalization()
if (released.await(POLL_MILLIS, TimeUnit.MILLISECONDS)) return true
System.gc()
}
return released.count == 0L
}
Expand Down
6 changes: 6 additions & 0 deletions bindings/rust/crates/maplibre-native-ffi/src/render/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1937,6 +1937,10 @@ impl OpenGLTestContext {
}
}

fn clear_gl_errors(&self) {
while unsafe { self.gl.get_error() } != gl_api::NO_ERROR {}
}

/// Reads the surface this context presents to.
///
/// The browser reads the context's own drawing buffer, which is what its
Expand Down Expand Up @@ -1994,6 +1998,7 @@ impl OpenGLBorrowedTexture {
height: u32,
) -> std::result::Result<gl_api::NativeTexture, Box<dyn StdError>> {
context.make_current()?;
context.clear_gl_errors();
let texture = unsafe {
let texture = context.gl.create_texture()?;
context.gl.bind_texture(gl_api::TEXTURE_2D, Some(texture));
Expand Down Expand Up @@ -2069,6 +2074,7 @@ impl OpenGLBorrowedTexture {

fn read_rgba(&self) -> std::result::Result<Vec<u8>, Box<dyn StdError>> {
self.context.make_current()?;
self.context.clear_gl_errors();
let mut pixels = vec![0_u8; self.width as usize * self.height as usize * 4];
let texture = self.texture.ok_or("borrowed texture has been deleted")?;
unsafe {
Expand Down
7 changes: 6 additions & 1 deletion build.zig
Original file line number Diff line number Diff line change
Expand Up @@ -630,14 +630,18 @@ fn addAndroidTestRunStep(
tests: []const *std.Build.Step.Compile,
native_install_dir: std.Build.LazyPath,
android_runner: std.Build.LazyPath,
emulator_api: ?[]const u8,
) *std.Build.Step.Run {
const run_tests = b.addSystemCommand(&.{
"bash",
android_runner.getPath(b),
"180",
installPath(b, native_install_dir, "lib/libmaplibre-native-c.so").getPath(b),
"--",
});
if (emulator_api) |api| {
run_tests.addArgs(&.{ "--api", api });
}
run_tests.addArg("--");
for (tests) |test_executable| {
run_tests.addArtifactArg(test_executable);
}
Expand Down Expand Up @@ -709,6 +713,7 @@ pub fn build(b: *std.Build) void {
&test_compiles,
options.native_install_dir,
b.path("scripts/run-android-emulator-test.sh"),
if (options.target.result.cpu.arch == .x86_64 and options.render_backend == .opengl) "26" else null,
);
test_step.dependOn(&run_tests.step);
} else {
Expand Down
9 changes: 8 additions & 1 deletion mise.toml
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,15 @@ elif [[ "$usage_preset" == ohos-x64-egl || "$usage_preset" == android-x64-* ]];
"$OHOS_SDK_NATIVE/llvm/lib/x86_64-linux-ohos/libc++_shared.so" \
-- "$MISE_MONOREPO_ROOT/build/$usage_preset/mln_ffi_c_api_tests"
fi
emulator_args=()
if [[ "$usage_preset" == android-x64-egl ]]; then
mise run //:android-emulator:boot x86_64 --api 26
Comment thread
sargunv marked this conversation as resolved.
emulator_args+=(--api 26)
fi
exec "$MISE_MONOREPO_ROOT/scripts/run-android-emulator-test.sh" \
300 \
"$MISE_MONOREPO_ROOT/build/$usage_preset/install/lib/libmaplibre-native-c.so" \
${emulator_args[@]+"${emulator_args[@]}"} \
-- "$MISE_MONOREPO_ROOT/build/$usage_preset/mln_ffi_c_api_tests"
fi
ctest --preset "$usage_preset"
Expand All @@ -364,14 +370,15 @@ usage = '''
arg "[abi]" default="x86_64" {
choices "arm64-v8a" "x86_64"
}
flag "--api <api>" default="{{vars.android_system_image_api}}"
'''
# sdkmanager and avdmanager run on a JVM. The emulator and its system image are
# not part of what .mise/bin/sync-android-packages installs, so asking for a
# boot is what accepts their licenses and pays for their download.
tools = { "core:java" = "{{vars.java_version}}" }
run = '''
"$MISE_MONOREPO_ROOT/scripts/boot-android-emulator.sh" \
"{{vars.android_system_image_api}}" \
"$usage_api" \
"$usage_abi"
'''

Expand Down
108 changes: 108 additions & 0 deletions patches/maplibre-native/0004-opengl-valid-api-calls.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
diff --git a/src/mln/gl/context.cpp b/src/mln/gl/context.cpp
index 02efd92..be82bff 100644
--- a/src/mln/gl/context.cpp
+++ b/src/mln/gl/context.cpp
@@ -151,10 +151,26 @@ void Context::endFrame() {
void Context::initializeExtensions(const std::function<gl::ProcAddress(const char*)>& getProcAddress) {
MLN_TRACE_FUNC();

- if (const auto* extensions = reinterpret_cast<const char*>(MBGL_CHECK_ERROR(glGetString(GL_EXTENSIONS)))) {
+ std::string extensions;
+#if defined(__ANDROID__) || defined(__EMSCRIPTEN__) || defined(__OHOS__)
+ GLint extensionCount = 0;
+ MBGL_CHECK_ERROR(glGetIntegerv(GL_NUM_EXTENSIONS, &extensionCount));
+ for (GLint index = 0; index < extensionCount; ++index) {
+ if (const auto* extension = reinterpret_cast<const char*>(MBGL_CHECK_ERROR(glGetStringi(GL_EXTENSIONS, index)))) {
Comment thread
sargunv marked this conversation as resolved.
+ extensions.append(extension);
+ extensions.push_back(' ');
+ }
+ }
+#else
+ if (const auto* extensionList = reinterpret_cast<const char*>(MBGL_CHECK_ERROR(glGetString(GL_EXTENSIONS)))) {
+ extensions = extensionList;
+ }
+#endif
+
+ if (!extensions.empty()) {
auto fn = [&](std::initializer_list<std::pair<const char*, const char*>> probes) -> ProcAddress {
for (auto probe : probes) {
- if (strstr(extensions, probe.first) != nullptr) {
+ if (extensions.find(probe.first) != std::string::npos) {
if (ProcAddress ptr = getProcAddress(probe.second)) {
return ptr;
}
diff --git a/src/mln/gl/resource_pool.cpp b/src/mln/gl/resource_pool.cpp
index 5c9fb64..df311df 100644
--- a/src/mln/gl/resource_pool.cpp
+++ b/src/mln/gl/resource_pool.cpp
@@ -145,15 +145,18 @@ TextureID Texture2DPool::allocateGLMemory(const Texture2DDesc& desc) {
// Bind to TU 0 and upload
context->activeTextureUnit = 0;
context->texture[0] = id;
- MBGL_CHECK_ERROR(glTexImage2D(GL_TEXTURE_2D,
- 0,
- Enum<gfx::TexturePixelType>::sizedFor(desc.pixelFormat, desc.channelType),
- desc.size.width,
- desc.size.height,
- 0,
- Enum<gfx::TexturePixelType>::to(desc.pixelFormat),
- Enum<gfx::TextureChannelDataType>::to(desc.channelType),
- nullptr));
+ // Attribute only errors from this allocation to allocation failure.
+ while (glGetError() != GL_NO_ERROR) {
+ }
+ glTexImage2D(GL_TEXTURE_2D,
+ 0,
+ Enum<gfx::TexturePixelType>::sizedFor(desc.pixelFormat, desc.channelType),
+ desc.size.width,
+ desc.size.height,
+ 0,
+ Enum<gfx::TexturePixelType>::to(desc.pixelFormat),
+ Enum<gfx::TextureChannelDataType>::to(desc.channelType),
+ nullptr);
if (glGetError()) {
throw std::bad_alloc();
}
diff --git a/src/mln/gl/uniform_buffer_gl.cpp b/src/mln/gl/uniform_buffer_gl.cpp
index 3ebafe8..0797706 100644
--- a/src/mln/gl/uniform_buffer_gl.cpp
+++ b/src/mln/gl/uniform_buffer_gl.cpp
@@ -89,9 +89,9 @@ UniformBufferGL::UniformBufferGL(const UniformBufferGL& other)
managedBuffer.allocate(other.managedBuffer.getContents().data(), other.size);
} else {
MBGL_CHECK_ERROR(glGenBuffers(1, &localID));
- MBGL_CHECK_ERROR(glCopyBufferSubData(other.localID, localID, 0, 0, size));
MBGL_CHECK_ERROR(glBindBuffer(GL_COPY_READ_BUFFER, other.localID));
MBGL_CHECK_ERROR(glBindBuffer(GL_COPY_WRITE_BUFFER, localID));
+ MBGL_CHECK_ERROR(glBufferData(GL_COPY_WRITE_BUFFER, size, nullptr, GL_DYNAMIC_DRAW));
MBGL_CHECK_ERROR(glCopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER, 0, 0, size));
Comment thread
sargunv marked this conversation as resolved.
}
}
diff --git a/src/mln/gl/upload_pass.cpp b/src/mln/gl/upload_pass.cpp
index 2eb6ef7..3940ae2 100644
--- a/src/mln/gl/upload_pass.cpp
+++ b/src/mln/gl/upload_pass.cpp
@@ -35,7 +35,10 @@ std::unique_ptr<gfx::VertexBufferResource> UploadPass::createVertexBufferResourc
// NOLINTNEXTLINE(performance-move-const-arg)
UniqueBuffer result{std::move(id), {commandEncoder.context}};
commandEncoder.context.vertexBuffer = result;
- MBGL_CHECK_ERROR(glBufferData(GL_ARRAY_BUFFER, size, data, Enum<gfx::BufferUsageType>::to(usage)));
+ // Attribute only errors from this allocation to allocation failure.
+ while (glGetError() != GL_NO_ERROR) {
+ }
+ glBufferData(GL_ARRAY_BUFFER, size, data, Enum<gfx::BufferUsageType>::to(usage));
if (glGetError()) {
throw std::bad_alloc();
}
@@ -63,7 +66,10 @@ std::unique_ptr<gfx::IndexBufferResource> UploadPass::createIndexBufferResource(
UniqueBuffer result{std::move(id), {commandEncoder.context}};
commandEncoder.context.bindVertexArray = 0;
commandEncoder.context.globalVertexArrayState.indexBuffer = result;
- MBGL_CHECK_ERROR(glBufferData(GL_ELEMENT_ARRAY_BUFFER, size, data, Enum<gfx::BufferUsageType>::to(usage)));
+ // Attribute only errors from this allocation to allocation failure.
+ while (glGetError() != GL_NO_ERROR) {
+ }
+ glBufferData(GL_ELEMENT_ARRAY_BUFFER, size, data, Enum<gfx::BufferUsageType>::to(usage));
if (glGetError()) {
throw std::bad_alloc();
}
6 changes: 6 additions & 0 deletions patches/maplibre-native/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ resources whose paths contain spaces or non-ASCII characters.
uses it to bound one pump's drain; the budget logic stays on the C API side, and
an unset gate keeps upstream behavior.

`0004-opengl-valid-api-calls.patch` uses indexed extension enumeration on OpenGL
ES, allocates storage before copying a uniform buffer, and isolates allocation
errors from earlier OpenGL calls. This prevents strict implementations and the
API 26 Android emulator from turning stale errors into false allocation
failures.

Drop a patch once the pin moves to a commit that carries it. The sync checks out
the pinned commit with `--force`, so it discards whatever the last sync applied
before applying the list again. A pin bump, an edit to a patch, and a dropped
Expand Down
4 changes: 2 additions & 2 deletions scripts/boot-android-emulator.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ case "$image_arch" in
esac
image="system-images;android-$api;default;$image_arch"
serial=emulator-5554
avd_name="mln-ffi-${image_arch//_/-}"
avd_name="mln-ffi-api-$api-${image_arch//_/-}"
sdk_root="${ANDROID_HOME:?ANDROID_HOME must point at an Android SDK}"
state_root="$MISE_MONOREPO_ROOT/build/android-emulator"
state_dir="$state_root/$image_arch"
state_dir="$state_root/$avd_name"
pid_file="$state_dir/emulator.pid"
log_file="$state_dir/emulator.log"
# The AVD lives in the build tree rather than the user's ~/.android, so a
Expand Down
19 changes: 16 additions & 3 deletions scripts/run-android-emulator-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,18 @@
set -euo pipefail

if [[ $# -lt 3 ]]; then
echo "usage: $0 <timeout-seconds> <native-library> [test-argument ...] -- <test-executable ...>" >&2
echo "usage: $0 <timeout-seconds> <native-library> [--api <api>] [test-argument ...] -- <test-executable ...>" >&2
exit 2
fi

timeout_seconds=$1
native_library=$2
shift 2
emulator_api=
if [[ ${1:-} == --api ]]; then
emulator_api=${2:?--api requires an Android API level}
shift 2
fi
test_arguments=()
while (($#)) && [[ $1 != -- ]]; do
test_arguments+=("$1")
Expand Down Expand Up @@ -41,17 +46,25 @@ if [[ ! "$timeout_seconds" =~ ^[0-9]+$ ]]; then
echo "Invalid timeout: $timeout_seconds" >&2
exit 2
fi
if [[ -n "$emulator_api" && ! "$emulator_api" =~ ^[0-9]+$ ]]; then
echo "Invalid Android API level: $emulator_api" >&2
exit 2
fi
if [[ -n "$fixture_dir" && ! -d "$fixture_dir" ]]; then
echo "Android emulator fixture directory does not exist: $fixture_dir" >&2
exit 2
fi

# platform-tools arrives with the first boot, so a missing adb means boot, not
# failure.
if [[ ! -x "$adb" ]] ||
if [[ -n "$emulator_api" ]] || [[ ! -x "$adb" ]] ||
! "$adb" -s "$serial" shell getprop sys.boot_completed 2>/dev/null |
tr -d '\r' | grep -qx 1; then
mise run //:android-emulator:boot
emulator_args=(x86_64)
if [[ -n "$emulator_api" ]]; then
emulator_args+=(--api "$emulator_api")
fi
mise run //:android-emulator:boot "${emulator_args[@]}"
Comment thread
sargunv marked this conversation as resolved.
fi

# The shell user may execute what it owns under /data/local/tmp. The Android
Expand Down
Loading
Loading