[dotCooCoo]

What and why

Revolve turns a profile into a 3D shape: the help page says it "Spins the object around an axis to create a circular 3D shape" (3D and Materials panel options), and the Classic help says "Revolving sweeps a path around the global Y axis to create a 3D object" (Create 3D objects). Revolve sorted its faces by the depth of each face's middle only, so a tilted profile with long edges, or a profile of several contours such as the outline of a dashed stroke, could draw a nearer face under a farther one where the two overlap. Extrude & Bevel and Inflate already order a shape's overlapping fill and stroke faces by their depths where they overlap when a stroke leaves gaps, such as a dashed stroke (#7154); this change runs that ordering over every pair of overlapping Revolve faces. Ordering faces by their depths at the overlap is VectorCraft's own rule; the help pages do not describe a drawing order.

• project_face (crates/three-d/src/lib.rs) gives each revolved Face its view-space plane, with the same 0.001 pt flatness and near-eye checks Extrude & Bevel's facets use; a face with a corner off its plane or at the eye has none. Facet::of_face (crates/three-d/src/solid.rs) keeps the plane, so Revolve (Classic)'s facets carry it too.
• solids::surface_order (crates/effects/src/solids.rs) runs Extrude & Bevel's local_order over one Revolve surface: when two overlapping faces are drawn the wrong way round, every pair of overlapping faces with a convex polygon and a plane goes in the order of its depths at the middle of the overlap, within the existing 4,000,000-comparison limit and its cycle handling. A surface already in the right order keeps it.
• revolve::surface (crates/effects/src/revolve.rs) puts the faces of Revolve and the facets of Revolve (Classic) in that order before graphics, visibility trimming and art. A group's members, sorted together by depth in group_art, go through the same order across members. In a group, faces already trimmed to their visible parts carry no plane and take no part in that comparison.
• docs/revolve.md, docs/effects-parity.md and docs/target-app-parity.md describe the order.

This changes the order Revolve draws a surface's faces in only when the old order drew a nearer face under a farther one. On such a surface the faces are reordered as a whole, so a face without a plane or with a non-convex polygon can also move.

How it was tested

New tests, each failing without the change:

• solids::tests_revolve_order::a_revolved_surface_draws_the_nearer_of_two_overlapping_faces_last (vectorcraft-effects): a vase profile of straight edges, turned 270° with a 12 pt offset and tilted 30° about X, has a face drawn before a farther face it overlaps. With the change no pair is drawn the wrong way round, on the object and as a member of a revolved group.
• solids::tests_revolve_order::a_revolved_groups_members_draw_the_nearer_of_two_overlapping_faces_last: five 14 × 20 pt rectangles as group members, revolved together, draw no pair of faces of one member or of two the wrong way round.
• solids::tests_revolve_order::a_classic_revolved_surface_draws_the_nearer_of_two_overlapping_facets_last: the same rectangles as one compound path under Revolve (Classic), tilted 60° in perspective.
• solids::tests_revolve_order::surface_order_puts_the_nearer_face_where_they_overlap_last: synthetic planes; the face nearer the viewer at the overlap goes last, a face without a plane or without an overlap keeps its place, and equal depths keep their order.
• tests::a_revolved_face_measures_its_depth_on_its_plane (vectorcraft-three-d): a revolved face's plane gives each corner's view-space depth, with and without perspective, and a twisted face or one with a corner past the eye has no plane.

solids::tests_revolve_order::a_surface_already_in_order_keeps_it checks that a vase seen straight on keeps its order by depth; it passes with and without the change.

Focused checks on the final tree, on Linux, on main at e56d1c7 (the branch merges cleanly with it):

• cargo fmt --all -- --check
• cargo clippy -p vectorcraft-three-d -p vectorcraft-effects --all-targets -- -D warnings
• cargo check --workspace --tests
• cargo test -p vectorcraft-effects --lib tests_revolve_order: 5 passed
• cargo test -p vectorcraft-three-d --lib a_revolved_face_measures_its_depth_on_its_plane: 1 passed
• cargo test -p vectorcraft-effects --lib revolve: 60 passed
• cargo xtask brands, cargo xtask layers (30 crates) and cargo xtask assets (337 files)
• cargo xtask ci runs in this pull request's CI.

Checklist

• cargo xtask ci passes (fmt, clippy, tests, assets, brands, layers, wasm).
• No panics in shipped code: no unwrap(), expect(), panic!, unreachable!, todo! or
  unimplemented! outside tests, and no unchecked indexing on data from files, parameters or
  messages. Errors are returned as Result and shown to the user
  (why and how).
• Clean-room: nothing copied from Adobe products or GPL/AGPL code (AGENTS.md).
• Every new asset has a row in ASSETS.md.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 51 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: storytold/vectorcraft
• Branch: main
• File: .coderabbit.yaml
• Commit: e56d1c7
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 16937f5e-c280-4ab1-833b-9a89c02ff3ac

Commits

Reviewing files that changed from the base of the PR and between e56d1c7 and 96d5cd7.

Files selected for processing: 10

• crates/effects/src/revolve.rs
• crates/effects/src/solids.rs
• crates/effects/src/solids/tests_revolve_order.rs
• crates/three-d/src/lib.rs
• crates/three-d/src/solid.rs
• crates/three-d/src/tests.rs
• crates/three-d/src/visibility.rs
• docs/effects-parity.md
• docs/revolve.md
• docs/target-app-parity.md

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[bflatastic]

Merged, thanks @dotCooCoo! It passed its CI checks.