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.

Sign up for GitHub

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

Jump to bottom

Add support for text-variable-anchor-offset property #2921

Merged
merged 48 commits into from
Oct 28, 2024
Merged

Add support for text-variable-anchor-offset property #2921

merged 48 commits into from
Oct 28, 2024

Conversation

wangyingfang
Copy link
Collaborator

@wangyingfang wangyingfang commented Oct 9, 2024 •
edited by louwers
Loading

Closes #2358. Support text-variable-anchor-offset property

Yingfang Wang and others added 22 commits August 23, 2024 19:33
add variableAnchorOffsetCollection data type for text-variable-anchor…
75362aa 
…-offset property
Merge branch 'main' into text-variable-anchor-offset
b1557f4
revert change in placement
06619f3
use text-variable-anchor-offset in placement
7096c76
fix failed render test
f7fe15a
change map to vector
7688166
add test case
ae701a2
build pass on Android
ccc68bd
@random3940
WIP: coersion when using data binding
38732fe
@random3940
WIP: NSExpressions
ebf2f58
update code to pass render test
108002b
@random3940
WIP: More NSExpressions
1dd1ec3
update expected images
f6334d1
@random3940
WIP
f48fc21
fix unit build and update expected images
2282396
@random3940
Minor clean-up
4c4a61e
@random3940
Fix node.js build
2464724
@random3940
Streamline VariableAnchorOffsetCollection API
3156ef5
Merge branch 'main' of https://github.com/wangyingfang/maplibre-native …
ea9c411 
…into text-variable-anchor-offset
update cache style db
d23b90d
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
7f19db2 
for more information, see https://pre-commit.ci
Merge branch 'main' of https://github.com/wangyingfang/maplibre-native …
e9e8ac4 
…into text-variable-anchor-offset
random3940
random3940 reviewed Oct 9, 2024
View reviewed changes
include/mbgl/style/layers/symbol_layer.hpp Outdated Show resolved Hide resolved
scripts/generate-style-code.mjs Show resolved Hide resolved
src/mbgl/layout/symbol_instance.cpp Show resolved Hide resolved
src/mbgl/layout/symbol_layout.cpp Show resolved Hide resolved
src/mbgl/renderer/buckets/symbol_bucket.cpp Outdated Show resolved Hide resolved
src/mbgl/style/expression/value.cpp Show resolved Hide resolved
src/mbgl/layout/symbol_instance.hpp Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 10, 2024 •
edited
Loading

Bloaty Results (iOS) 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.8%  +110Ki  +1.0%  +128Ki    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results-ios/pr-2921-compared-to-main.txt

@wangyingfang
Copy link
Collaborator Author

@louwers @random3940 please help review again.

@random3940
Copy link
Collaborator

LGTM!

@louwers
Copy link
Collaborator

louwers commented Oct 22, 2024

Thank you, having another look over it.

louwers
louwers approved these changes Oct 23, 2024
View reviewed changes
Copy link
Collaborator

@louwers louwers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this excellent contribution!

I left a few more small comments, let's merge after those are addressed.

@random3940 Feel free to also formally approve it using GitHub's UI.

@wangyingfang
Copy link
Collaborator Author

wangyingfang commented Oct 24, 2024 •
edited
Loading

@louwers @random3940 I resolve the latest comments, please help review again.

@random3940
Copy link
Collaborator

@louwers, iOS C++ test fails with this error:

<Code>SignatureDoesNotMatch</Code>
<Message>The request signature we calculated does not match the signature you provided. Check your key and signing method.</Message>

It looks like some CI glitch to me. Just in case I've re-ran tests from this branch locally and they pass.

@louwers
Copy link
Collaborator

louwers commented Oct 24, 2024

I will take a look at it.

@louwers louwers changed the title Text variable anchor offset Add support for text-variable-anchor-offset property Oct 27, 2024
@louwers louwers mentioned this pull request Oct 27, 2024
Open
@wangyingfang wangyingfang reopened this Oct 28, 2024
@louwers louwers merged commit 1a63194 into maplibre:main Oct 28, 2024
39 checks passed
@louwers louwers mentioned this pull request Oct 30, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@random3940 random3940 random3940 approved these changes

@louwers louwers louwers approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request js-parity
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support text-variable-anchor-offset property
3 participants
@wangyingfang @random3940 @louwers