[PROTOCONV] Convert the Kotlin code to Proto #1886
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will cause syntax errors across the codebase, but will mean that converting code in a file should clear those errors. As an example, replacing all instances of the Serde NodeStyle with the Proto NodeStyle means that this line of code is now an error:
<!-- start git-machete generated --> # Based on PR #1879 ## Chain of upstream PRs as of 2024-12-18 * PR #1879: `feature/protoconv` ← `wb/froeht/swap-dcd-to-proto` * **PR #1874 (THIS ONE)**: `wb/froeht/swap-dcd-to-proto` ← `wb/froeht/test-convert` <!-- end git-machete generated --> If you open SquooshLayout in Android Studio you'll see that there's no errors listed. Of course everything else needs to be converted too, so nothing will compile.
This was referenced Dec 18, 2024
<!-- start git-machete generated --> # Based on PR #1886 ## Chain of upstream PRs as of 2024-12-18 * PR #1886: `main` ← `feature/protoconv` * **PR #1888 (THIS ONE)**: `feature/protoconv` ← `wb/froeht/convert-variablemanager` <!-- end git-machete generated --> There's still a few syntax issues because the `toColor()` function hasn't been converted yet. Also included a configuration file for the CamelCase plugin that disables the naming schemes that we aren't using.
MathUtils calls one Utils api. Double check when #1916 merged.
Also need to update ProtoUtils StrokeWeight functions. Fixes: #1889
Fix mergeStyles() function to take values from base style by default, and from override view style if set. Fix opacity value when rendering.
|
Snapshot diff report vs base branch: main |
Merged
The FrameRender change is not related. Just feel cleaner to put it that way.
timothyfroehlich
requested review from
rylin8,
yiqunw700 and
iamralpht
and removed request for
yiqunw700
yiqunw700
approved these changes
Jan 8, 2025
This was referenced Jan 9, 2025
Remove unused code Change builders to copy()
<!-- start git-machete generated --> # Based on PR #1886 ## Chain of upstream PRs as of 2025-01-09 * PR #1886: `main` ← `feature/protoconv` * **PR #1964 (THIS ONE)**: `feature/protoconv` ← `wb/froeht/improve-docserializationtest` <!-- end git-machete generated --> This change modifies the test setup for `DocSerializationTest` to improve its robustness and coverage. - Removes `showStandardStreams = true` from the roborazzi gradle config, to quiet down the amount of console spam - Adds the validation assets directory to the test resources for the common module. - Replaces the single `loadSaveLoadHelloWorld` test with a parameterized test `LoadSaveLoadAllDocsTest` to make it easier to see which docs aren't passing - Removes the `VariantAnimationTimelineTestDoc_vJRf4zxY4QX4zzSSUd1nJ5.dcf` file. The test is disabled right now ( #1945 ) so the dcf wasn't being updated and was failing to decode.
…conversion code (#1965) With this change we're able to fully remove serde-reflection from the code base. This involved a decent amount of refactoring and cleanup of old work to get things right. My goal with the refactoring was to keep the packages cleanly separated and to work towards having the ability to build a DesignCompose app without including anything related to Live Update. Ultimately I could see the dc_jni crate being split into the layout and fetch portions, which would compile into their own separate libraries, and merged in with figma_import and layout. Some of the messages related to both the doc fetching and layout processing were a bit scattered and were declared in crates that didn't actually use them. Before this change the `dc_jni` crate didn't have any of its own proto messages, it was pulling the ones it needed (Layout conversion related) from other crates (both `dc_bundle` and `layout`). The new Convert Request/Response messages would only be used by `dc_jni`, so it made sense to add a build.rs file to it so that the generated messages would be native to that crate, rather than being part of an unrelated crate. While making that change it made sense to rearrange the proto messages related to the android interface, so I moved the layout related messages into a `layout_interface` proto package and put the new Convert messages into the `live_update` package. In addition, the layout code had really only been half converted to proto, we were still parsing them and then converting them into the original Rust structs. That's all cleaned up now. We had been manually generating a json string which was decoded into the ConvertRequest on the rust side, so I converted the ConvertRequest to Proto and removed the json code. It didn't make sense to use proto on one side and not the other. It'd probably be cleaner to change the `jniFetchDoc` method to just send a single proto message instead of the ConvertRequest, doc_id, version_id and proxy configuration, but that can be cleaned up another day. I also decided not to convert the image_session from json to proto because we don't actually do anything with it on the Kotlin side other than writing it into the .dcf. I'm simply including the json in the proto message and removing the redundant storage of the session's bytes in the docContent class. Admittedly this PR is bigger than it strictly needed to be. I can split it into some smaller PRs if requested. I'm sure there's some additional cleanup that could be done as well, including simplifying the `jniFetchDoc` function and turning ProxyConfig into a proto struct.
iamralpht
approved these changes
Jan 13, 2025
Fixes #1945: Add functions to VariantTransitionContext that allow access to the view names.
rylin8
approved these changes
Jan 13, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.