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

TextFit: Internal review #1

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion .github/actions/aws-device-farm-run/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ runs:
run: |
arn="$(aws devicefarm schedule-run \
--project-arn ${{ inputs.AWS_DEVICE_FARM_PROJECT_ARN }} \
--name "MapLibre Native ${{ matrix.test.name }}" \
--name "MapLibre Native ${{ inputs.name }}" \
--app-arn ${{ env.app_arn }} \
--device-pool-arn ${{ inputs.AWS_DEVICE_FARM_DEVICE_POOL_ARN }} \
--test type=${{ inputs.testType }},testPackageArn=${{ env.test_package_arn }}${{ inputs.testFilter && ',filter=' }}${{ inputs.testFilter }}${{ inputs.testSpecArn && ',testSpecArn=' }}${{ inputs.testSpecArn }} \
Expand Down
39 changes: 36 additions & 3 deletions .github/workflows/android-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,27 @@ jobs:
./test/android/app-release-androidTest.apk

android-build-render-test:
strategy:
fail-fast: false
matrix:
flavor: [opengl, vulkan]
runs-on: ubuntu-latest
needs:
- pre_job
if: needs.pre_job.outputs.should_skip != 'true'
steps:
- name: Free Disk Space (Ubuntu)
if: startsWith(runner.name, 'GitHub Actions')
uses: jlumbroso/free-disk-space@main
with:
tool-cache: false
android: false
dotnet: true
haskell: true
large-packages: true
docker-images: true
swap-storage: false

- uses: actions/checkout@v4
with:
submodules: recursive
Expand All @@ -248,12 +264,29 @@ jobs:

- name: Build Render Test App
run: |
./gradlew assemble assembleAndroidTest
cp app/build/outputs/apk/release/app-release.apk RenderTestsApp.apk
cp app/build/outputs/apk/androidTest/release/app-release-androidTest.apk RenderTests.apk
./gradlew assemble${{ matrix.flavor }} assemble${{ matrix.flavor }}AndroidTest
cp app/build/outputs/apk/${{ matrix.flavor }}/release/app-${{ matrix.flavor }}-release.apk RenderTestsApp-${{ matrix.flavor }}.apk
cp app/build/outputs/apk/androidTest/${{ matrix.flavor }}/release/app-${{ matrix.flavor }}-release-androidTest.apk RenderTests-${{ matrix.flavor }}.apk
working-directory: ./render-test/android

- name: Store Render Test .apk files
uses: actions/upload-artifact@v4
with:
name: android-render-tests-${{ matrix.flavor }}
if-no-files-found: error
path: |
./render-test/android/RenderTestsApp-${{ matrix.flavor }}.apk
./render-test/android/RenderTests-${{ matrix.flavor }}.apk

# these two can be (and should be) removed once merged into main
- name: Copy render tests and render test app
if: matrix.flavor == 'opengl'
run: |
cp ./render-test/android/RenderTestsApp-opengl.apk ./render-test/android/RenderTestsApp.apk
cp ./render-test/android/RenderTests-opengl.apk ./render-test/android/RenderTests.apk

- name: Store Render Test .apk files
if: matrix.flavor == 'opengl'
uses: actions/upload-artifact@v4
with:
name: android-render-tests
Expand Down
16 changes: 12 additions & 4 deletions .github/workflows/android-device-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,18 @@ jobs:
matrix:
test: [
{
artifactName: android-render-tests,
testFile: RenderTests.apk,
appFile: RenderTestsApp.apk,
name: "Android Render Tests",
artifactName: android-render-tests-opengl,
testFile: RenderTests-opengl.apk,
appFile: RenderTestsApp-opengl.apk,
name: "Android Render Tests (OpenGL)",
# Google Pixel 7 Pro
devicePool: "arn:aws:devicefarm:us-west-2:373521797162:devicepool:20687d72-0e46-403e-8f03-0941850665bc/9692fe7f-86a9-4ecc-908f-175600968564"
},
{
artifactName: android-render-tests-vulkan,
testFile: RenderTests-vulkan.apk,
appFile: RenderTestsApp-vulkan.apk,
name: "Android Render Tests (Vulkan)",
# Google Pixel 7 Pro
devicePool: "arn:aws:devicefarm:us-west-2:373521797162:devicepool:20687d72-0e46-403e-8f03-0941850665bc/9692fe7f-86a9-4ecc-908f-175600968564"
},
Expand Down
37 changes: 35 additions & 2 deletions .github/workflows/android-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@ name: android-release

on:
workflow_dispatch:
inputs:
renderer:
description: 'Select renderering backend'
required: true
default: 'OpenGL'
type: choice
options:
- OpenGL
- Vulkan

jobs:
build:
Expand All @@ -19,6 +28,15 @@ jobs:
submodules: recursive
fetch-depth: 0

- name: Map renderer input
id: backend_lowercase
run: |
if [ "${{ github.event.inputs.renderer }}" = "OpenGL" ]; then
echo "backend_lowercase=drawable" >> "$GITHUB_ENV"
elif [ "${{ github.event.inputs.renderer }}" = "Vulkan" ]; then
echo "backend_lowercase=vulkan" >> "$GITHUB_ENV"
fi

- run: echo "cmake.dir=$(dirname "$(dirname "$(command -v cmake)")")" >> local.properties

- uses: actions/setup-java@v4
Expand Down Expand Up @@ -97,6 +115,8 @@ jobs:

- name: Build package
run: make apackage
env:
RENDERER: ${{ env.backend_lowercase }}

- name: Build release Test App
run: |
Expand All @@ -119,6 +139,18 @@ jobs:
echo version_tag="$( git describe --tags --match=android-v*.*.* --abbrev=0 )" >> "$GITHUB_OUTPUT"
shell: bash

- name: Check if version is valid semver
id: check_version
run: |
version_tag="${{ steps.prepare_release.outputs.version_tag }}"
if [[ $version_tag =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
echo "Valid semver: $version_tag"
echo "prerelease=false" >> "$GITHUB_ENV"
else
echo "Invalid semver: $version_tag"
echo "prerelease=true" >> "$GITHUB_ENV"
fi

- name: Create release
id: create_release
uses: actions/create-release@v1
Expand All @@ -129,7 +161,7 @@ jobs:
release_name: ${{steps.prepare_release.outputs.version_tag }}
body_path: ${{ steps.prepare_release.outputs.release_notes }}
draft: false
prerelease: false
prerelease: ${{ env.prerelease }}

- name: Upload aar
id: upload-release-asset
Expand All @@ -138,7 +170,7 @@ jobs:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
upload_url: ${{ steps.create_release.outputs.upload_url }}
asset_path: platform/android/MapLibreAndroid/build/outputs/aar/MapLibreAndroid-drawable-release.aar
asset_path: platform/android/MapLibreAndroid/build/outputs/aar/MapLibreAndroid-${{ env.backend_lowercase }}-release.aar
asset_name: MapLibreAndroid-release.aar
asset_content_type: application/zip

Expand Down Expand Up @@ -172,3 +204,4 @@ jobs:
SIGNING_KEY_ID: ${{ secrets.SIGNING_KEY_ID }}
SIGNING_PASSWORD: ${{ secrets.SIGNING_PASSWORD }}
SONATYPE_STAGING_PROFILE_ID: ${{ secrets.SONATYPE_STAGING_PROFILE_ID }}
RENDERER: ${{ env.backend_lowercase }}
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ repos:
files: '.*\.(hpp|cpp|h)'
exclude: '(vendor/.*|darwin/include/mbgl/storage/reachability.h)'
- repo: https://github.com/keith/pre-commit-buildifier
rev: 7.1.2
rev: 7.3.1
hooks:
- id: buildifier
- repo: https://github.com/Mateusz-Grzelinski/actionlint-py
Expand Down
28 changes: 15 additions & 13 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ if (MLN_WITH_QT AND NOT CMAKE_OSX_DEPLOYMENT_TARGET)
set(CMAKE_OSX_DEPLOYMENT_TARGET 13.0)
endif()

if(WIN32 AND NOT MLN_WITH_QT)
if(WIN32 AND NOT MLN_WITH_QT AND NOT CMAKE_SYSTEM_NAME STREQUAL Android)
set(CMAKE_TOOLCHAIN_FILE ${CMAKE_CURRENT_LIST_DIR}/platform/windows/custom-toolchain.cmake)
endif()

Expand Down Expand Up @@ -252,6 +252,8 @@ list(APPEND INCLUDE_FILES
${PROJECT_SOURCE_DIR}/include/mbgl/layermanager/layer_factory.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/layermanager/layer_manager.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/layermanager/line_layer_factory.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/layers/location_indicator_layer.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/layermanager/location_indicator_layer_factory.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/layermanager/raster_layer_factory.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/layermanager/symbol_layer_factory.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/map/bound_options.hpp
Expand Down Expand Up @@ -503,6 +505,14 @@ list(APPEND SRC_FILES
${PROJECT_SOURCE_DIR}/src/mbgl/layermanager/line_layer_factory.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/layermanager/raster_layer_factory.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/layermanager/symbol_layer_factory.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/layermanager/location_indicator_layer_factory.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/renderer/layers/render_location_indicator_layer.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/renderer/layers/render_location_indicator_layer.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/layers/location_indicator_layer.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/layers/location_indicator_layer_impl.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/layers/location_indicator_layer_impl.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/layers/location_indicator_layer_properties.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/layers/location_indicator_layer_properties.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/layout/circle_layout.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/layout/clip_lines.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/layout/clip_lines.hpp
Expand Down Expand Up @@ -856,6 +866,8 @@ list(APPEND SRC_FILES
${PROJECT_SOURCE_DIR}/src/mbgl/layermanager/custom_layer_factory.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/layers/custom_layer_impl.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/layers/custom_layer_impl.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/renderer/layers/render_custom_layer.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/renderer/layers/render_custom_layer.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/text/bidi.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/text/check_max_angle.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/text/check_max_angle.hpp
Expand Down Expand Up @@ -1006,9 +1018,7 @@ if(MLN_WITH_OPENGL)
INCLUDE_FILES
${PROJECT_SOURCE_DIR}/include/mbgl/gl/renderable_resource.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/gl/renderer_backend.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/layermanager/location_indicator_layer_factory.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/platform/gl_functions.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/style/layers/location_indicator_layer.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/shaders/shader_source.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/shaders/shader_manifest.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/shaders/gl/shader_group_gl.hpp
Expand Down Expand Up @@ -1090,8 +1100,6 @@ if(MLN_WITH_OPENGL)
${PROJECT_SOURCE_DIR}/src/mbgl/gl/offscreen_texture.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/gl/offscreen_texture.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/gl/program.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/renderer/layers/render_custom_layer.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/renderer/layers/render_custom_layer.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/gl/render_pass.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/gl/render_pass.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/gl/renderbuffer_resource.cpp
Expand All @@ -1117,15 +1125,7 @@ if(MLN_WITH_OPENGL)
${PROJECT_SOURCE_DIR}/src/mbgl/gl/vertex_array.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/gl/vertex_buffer_resource.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/gl/vertex_buffer_resource.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/layermanager/location_indicator_layer_factory.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/platform/gl_functions.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/renderer/layers/render_location_indicator_layer.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/renderer/layers/render_location_indicator_layer.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/layers/location_indicator_layer.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/layers/location_indicator_layer_impl.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/layers/location_indicator_layer_impl.hpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/layers/location_indicator_layer_properties.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/style/layers/location_indicator_layer_properties.hpp
)

if(MLN_DRAWABLE_RENDERER)
Expand Down Expand Up @@ -1321,6 +1321,7 @@ if(MLN_WITH_VULKAN)
${PROJECT_SOURCE_DIR}/include/mbgl/shaders/vulkan/line.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/shaders/vulkan/raster.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/shaders/vulkan/symbol.hpp
${PROJECT_SOURCE_DIR}/include/mbgl/shaders/vulkan/widevector.hpp
)

list(APPEND
Expand Down Expand Up @@ -1355,6 +1356,7 @@ if(MLN_WITH_VULKAN)
${PROJECT_SOURCE_DIR}/src/mbgl/shaders/vulkan/line.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/shaders/vulkan/raster.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/shaders/vulkan/symbol.cpp
${PROJECT_SOURCE_DIR}/src/mbgl/shaders/vulkan/widevector.cpp
)
endif()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ void main() {

// these transformations used to be applied in the JS and native code bases.
// moved them into the shader for clarity and simplicity.
float u_width = 1.0;
float halfwidth = u_width / 2.0;
float outset = halfwidth + (halfwidth == 0.0 ? 0.0 : ANTIALIASING);

Expand Down
4 changes: 2 additions & 2 deletions include/mbgl/shaders/vulkan/background.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ struct ShaderSource<BuiltIn::BackgroundShader, gfx::Backend::Type::Vulkan> {

static constexpr auto vertex = R"(

layout(location = 0) in vec2 in_position;
layout(location = 0) in ivec2 in_position;

layout(set = 0, binding = 1) uniform BackgroundDrawableUBO {
mat4 matrix;
Expand Down Expand Up @@ -61,7 +61,7 @@ struct ShaderSource<BuiltIn::BackgroundPatternShader, gfx::Backend::Type::Vulkan

static constexpr auto vertex = R"(

layout(location = 0) in vec2 in_position;
layout(location = 0) in ivec2 in_position;

layout(set = 0, binding = 1) uniform BackgroundPatternDrawableUBO {
mat4 matrix;
Expand Down
2 changes: 1 addition & 1 deletion include/mbgl/shaders/vulkan/circle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ struct ShaderSource<BuiltIn::CircleShader, gfx::Backend::Type::Vulkan> {

static constexpr auto vertex = R"(

layout(location = 0) in vec2 in_position;
layout(location = 0) in ivec2 in_position;

#if !defined(HAS_UNIFORM_u_color)
layout(location = 1) in vec4 in_color;
Expand Down
2 changes: 1 addition & 1 deletion include/mbgl/shaders/vulkan/clipping_mask.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ struct ShaderSource<BuiltIn::ClippingMaskProgram, gfx::Backend::Type::Vulkan> {
static constexpr std::array<TextureInfo, 0> textures{};

static constexpr auto vertex = R"(
layout(location = 0) in vec2 position;
layout(location = 0) in ivec2 position;

layout(push_constant) uniform constants {
mat4 matrix;
Expand Down
16 changes: 8 additions & 8 deletions include/mbgl/shaders/vulkan/collision.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ struct ShaderSource<BuiltIn::CollisionBoxShader, gfx::Backend::Type::Vulkan> {

static constexpr auto vertex = R"(

layout(location = 0) in vec2 in_position;
layout(location = 1) in vec2 in_anchor_position;
layout(location = 2) in vec2 in_extrude;
layout(location = 3) in vec2 in_placed;
layout(location = 0) in ivec2 in_position;
layout(location = 1) in ivec2 in_anchor_position;
layout(location = 2) in ivec2 in_extrude;
layout(location = 3) in uvec2 in_placed;
layout(location = 4) in vec2 in_shift;

layout(set = 0, binding = 1) uniform CollisionBoxUBO {
Expand Down Expand Up @@ -98,10 +98,10 @@ struct ShaderSource<BuiltIn::CollisionCircleShader, gfx::Backend::Type::Vulkan>

static constexpr auto vertex = R"(

layout(location = 0) in vec2 in_position;
layout(location = 1) in vec2 in_anchor_position;
layout(location = 2) in vec2 in_extrude;
layout(location = 3) in vec2 in_placed;
layout(location = 0) in ivec2 in_position;
layout(location = 1) in ivec2 in_anchor_position;
layout(location = 2) in ivec2 in_extrude;
layout(location = 3) in uvec2 in_placed;

layout(set = 0, binding = 1) uniform CollisionCircleUBO {
mat4 matrix;
Expand Down
15 changes: 1 addition & 14 deletions include/mbgl/shaders/vulkan/debug.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ struct ShaderSource<BuiltIn::DebugShader, gfx::Backend::Type::Vulkan> {

static constexpr auto vertex = R"(

layout(location = 0) in vec2 in_position;
layout(location = 0) in ivec2 in_position;

layout(set = 0, binding = 1) uniform DebugUBO {
mat4 matrix;
Expand Down Expand Up @@ -59,18 +59,5 @@ void main() {
)";
};

template <>
struct ShaderSource<BuiltIn::WideVectorShader, gfx::Backend::Type::Vulkan> {
static constexpr const char* name = "WideVectorShader";

static constexpr std::array<UniformBlockInfo, 0> uniforms{};
static constexpr std::array<AttributeInfo, 0> attributes{};
static constexpr std::array<AttributeInfo, 0> instanceAttributes{};
static constexpr std::array<TextureInfo, 0> textures{};

static constexpr auto vertex = R"()";
static constexpr auto fragment = R"()";
};

} // namespace shaders
} // namespace mbgl
Loading