Skip to content

Preparing CLI encoder for public release #504

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

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

TimSylvester
Copy link
Collaborator

@TimSylvester TimSylvester commented Jul 9, 2025

  • Add support for prepending metadata to tiles.
  • Add support for encoding entire MBTiles and offline database files.
  • Add options to the CLI for various existing optional features:
    • pre-tessellation
    • Morton encoding
    • include IDs
    • outline tables
  • Add new options:
    • local vs. remote tessellation
    • compression
    • verbose output
  • Remove vectorized decoding to eliminate the dependency on jdk.incubator.vector
  • Refactor ConversionConfig per TODO
  • Fix a bunch of style warnings
  • Apply spotless formatting

@TimSylvester TimSylvester self-assigned this Jul 9, 2025
@Copilot Copilot AI review requested due to automatic review settings July 9, 2025 22:25
Copilot

This comment was marked as outdated.

@codecov-commenter
Copy link

codecov-commenter commented Jul 9, 2025

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.05%. Comparing base (1087072) to head (97d7e1b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
rust/mlt/src/decoder/decode.rs 66.66% 1 Missing ⚠️
rust/wasm-test/src/lib.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #504      +/-   ##
==========================================
- Coverage   41.89%   39.05%   -2.84%     
==========================================
  Files          14       14              
  Lines         950      868      -82     
  Branches      950      868      -82     
==========================================
- Hits          398      339      -59     
+ Misses        529      510      -19     
+ Partials       23       19       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

normalize getter naming
Remove experimental vectorized variations and `jdk.incubator.vector` dependency
Add command line options, refactor, fix warnings
@TimSylvester TimSylvester changed the title Expose options in the encoder CLI, remove vectorized variations Preparing CLI encoder for public release Jul 23, 2025
@TimSylvester TimSylvester requested a review from Copilot July 25, 2025 14:19
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR prepares the CLI encoder for public release by removing vectorized decoding dependencies and enhancing the CLI with comprehensive encoding options. The changes focus on improving usability and removing experimental features.

  • Removes vectorized decoding functionality and the dependency on jdk.incubator.vector
  • Refactors the configuration system and consolidates encoding options into a unified ConversionConfig
  • Adds comprehensive CLI options for MBTiles/offline database encoding, compression, tessellation, and metadata handling

Reviewed Changes

Copilot reviewed 32 out of 33 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
rust/wasm-test/src/lib.rs Updates string formatting to use newer Rust syntax
rust/mlt/src/metadata/tileset.rs Updates error message formatting to use newer Rust syntax
rust/mlt/src/decoder/decode.rs Updates print statement and error message formatting to use newer Rust syntax
java/src/test/java/com/mlt/decoder/vectorized/VectorizedDecodingUtilsTest.java Removes vectorized RLE test and fixes collection construction
java/src/test/java/com/mlt/decoder/MltDecoderTest2.java Adds null checks, fixes string comparison, and removes vectorized decoding
java/src/test/java/com/mlt/decoder/MltDecoderTest.java Removes vectorized test methods and cleans up imports
java/src/test/java/com/mlt/decoder/DecodingUtilsTest.java Removes unused imports and fixes assertion method
java/src/test/java/com/mlt/benchmarks/MltDecoderBenchmark.java Comments out vectorized decoding calls
java/src/test/java/com/mlt/benchmarks/CompressionBenchmarks.java Updates to use sequential decoding instead of vectorized
java/src/test/java/com/mlt/TestUtils.java Adds @SuppressWarnings annotation for unchecked cast
java/src/test/java/com/mlt/MltGenerator.java Refactors to use unified ConversionConfig constructor
java/src/main/java/com/mlt/decoder/vectorized/fastpfor/VectorFastPFOR.java Removes entire vectorized FastPFOR implementation
java/src/main/java/com/mlt/decoder/vectorized/VectorizedPropertyDecoder.java Removes entire vectorized property decoder
java/src/main/java/com/mlt/decoder/vectorized/VectorizedGeometryDecoder.java Removes entire vectorized geometry decoder
java/src/main/java/com/mlt/decoder/vectorized/VectorizedFloatDecoder.java Removes entire vectorized float decoder
java/src/main/java/com/mlt/decoder/vectorized/VectorizedDoubleDecoder.java Removes entire vectorized double decoder
java/src/main/java/com/mlt/decoder/vectorized/VectorizedDecodingUtils.java Removes vectorized methods and vector API dependencies
java/src/main/java/com/mlt/decoder/MltDecoder.java Removes vectorized decoding method and related functionality
java/src/main/java/com/mlt/converter/tessellation/TessellationUtils.java Adds support for local vs remote tessellation with URI parameter
java/src/main/java/com/mlt/converter/mvt/MvtUtils.java Adds @SuppressWarnings annotations for optional parameters
java/src/main/java/com/mlt/converter/encodings/GeometryEncoder.java Adds tessellation source parameter support
java/src/main/java/com/mlt/converter/encodings/EncodingUtils.java Removes vectorized FastPFOR encoding method
java/src/main/java/com/mlt/converter/RenderingOptimizedConversionConfig.java Removes entire specialized config class
java/src/main/java/com/mlt/converter/MltConverter.java Updates to use unified ConversionConfig and adds tessellation source support
java/src/main/java/com/mlt/converter/ConversionConfig.java Major refactoring to consolidate all configuration options into one class
java/src/main/java/com/mlt/cli/Timer.java Updates package name from tools to cli
java/src/main/java/com/mlt/cli/Meta.java Updates package name from tools to cli
java/src/main/java/com/mlt/cli/Encode.java Major expansion with comprehensive CLI options for encoding different file types
java/src/main/java/com/mlt/cli/Decode.java Updates package name and removes vectorized decoding support
java/src/main/java/com/mlt/cli/CliUtil.java Updates package name from tools to cli
java/src/jmh/java/com/mlt/EncodingsBenchmarks.java Removes vectorized decoding import
java/build.gradle Removes vector API dependencies and adds new dependencies for CLI functionality
Comments suppressed due to low confidence (1)

java/src/test/java/com/mlt/decoder/MltDecoderTest2.java:231

  • The error message should be more descriptive and helpful. Consider adding information about alternatives or migration path.
      throw new NotImplementedException("Vectorized decoding is not available");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants