Added commands for object transforming (translate, rotate and scale) #441

benjamaan476 · 2024-08-08T15:00:56Z

Added Commands to transform objects:

SetObject(Position/Rotation/Scale) will set the specified local value to the passed in argument
(Translate/Rotate/Scale)Object is a relative change based on the current local value. E.g. translate({5, 0, 0}) will move the object 5 units along the x-axis from say {11, 0, 0} to {16, 0, 0}. SetObjectPosition({5, 0, 0}) will set the position to {5, 0, 0}
SetObjectParent will set the parent object of an object. The object (child) will inherit its parent's local values and its values will be applied on top of those values

benjamaan476 · 2024-08-08T15:06:45Z

Don't merge - testing against the engine

codecov · 2024-08-08T15:07:49Z

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@d87e394). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
modeling-cmds/src/shared.rs	0.00%	2 Missing ⚠️
modeling-cmds/src/def_enum.rs	0.00%	1 Missing ⚠️
modeling-cmds/src/format/obj.rs	0.00%	1 Missing ⚠️
modeling-cmds/src/format/ply.rs	0.00%	1 Missing ⚠️
modeling-cmds/src/format/sldprt.rs	0.00%	1 Missing ⚠️
modeling-cmds/src/format/step.rs	0.00%	1 Missing ⚠️
modeling-cmds/src/format/stl.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #441   +/-   ##
=======================================
  Coverage        ?   25.33%           
=======================================
  Files           ?       35           
  Lines           ?     2096           
  Branches        ?        0           
=======================================
  Hits            ?      531           
  Misses          ?     1565           
  Partials        ?        0

Flag	Coverage Δ
unittests	`25.33% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

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

…ach transform type

lf94 · 2024-08-08T16:20:59Z

modeling-cmds/src/def_enum.rs

+        #[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)]
+        struct TransformBy<T> {
+            /// The scale, or rotation, or translation.
+            property: T,
+            /// If true, overwrite the previous value with this.
+            /// If false, the object will be scaled, or translated, or rotated, etc.
+            set: bool,
+        }
+
+        ///Set the transform of an object.
+        #[derive(
+            Clone, Debug, Deserialize, JsonSchema, Serialize, ModelingCmdVariantEmpty,
+        )]
+        pub struct SetObjectTransform
+        {
+            ///Id of the object whose transform is to be set
+            pub object_id: Uuid,
+            #[serde(default)]
+            translate: Option<TransformBy<Point3d>>,
+            #[serde(default)]
+            rotate: Option<TransformBy<Point3d>>,
+            #[serde(default)]
+            scale: Option<TransformBy<Point3d>>,
+        }


The following is just a suggestion/idea. We can further reduce bandwidth by instead sending a list of transforms. Otherwise looks great!

Suggested change

#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)]

struct TransformBy<T> {

/// The scale, or rotation, or translation.

property: T,

/// If true, overwrite the previous value with this.

/// If false, the object will be scaled, or translated, or rotated, etc.

set: bool,

}

///Set the transform of an object.

#[derive(

Clone, Debug, Deserialize, JsonSchema, Serialize, ModelingCmdVariantEmpty,

)]

pub struct SetObjectTransform

{

///Id of the object whose transform is to be set

pub object_id: Uuid,

#[serde(default)]

translate: Option<TransformBy<Point3d>>,

#[serde(default)]

rotate: Option<TransformBy<Point3d>>,

#[serde(default)]

scale: Option<TransformBy<Point3d>>,

}

enum TransformOperation {

Translate,

Rotate,

Scale

}

struct Transform<T> {

operation: TransformOperation,

value: T,

set: bool,

}

///Set the transform of an object.

#[derive(

Clone, Debug, Deserialize, JsonSchema, Serialize, ModelingCmdVariantEmpty,

)]

pub struct SetObjectTransform

{

///Id of the object whose transform is to be set

pub object_id: Uuid,

transforms: Vec<Transform<Point3D>>

}

I thought about this, but this allows you to send multiple transforms that overwrite each other, e.g. [(Translate, <0,0,0>), (Translate, <1,1,1>)]. So I think the current approach is better -- it limits you to sending at most one transform for a given property and the given project.

It could be desirable from a programmer or mathematician point of view to compose transforms in various ways, ways I cannot imagine - for example [(Translate, <0,0,0>), (Translate, <1,1,1>)] could be desirable if they are composing (have set: false). Otherwise I agree with you.

You are knocking on the door of a transform stack here. Which is where you specify a chain of transforms one after the other that get applied in that order. Typically comes along with the ability to push and pop transforms to a stack that lets you get pretty flexible

We're now agreed that Transform endpoint should take a list of transforms.

So these transforms will get applied one by one if more than one is provided? Is it up to the user to handle the complexity of the list? E.g. if two translates are side by side that could have just been one aggregate transform.

alteous · 2025-01-06T12:49:13Z

modeling-cmds/src/shared.rs

+    /// If false, the previous value will be modified.
+    /// E.g. when translating, `set=true` will set a new location,
+    /// and `set=false` will translate the current location by the given X/Y/Z.
+    pub set: bool,


Is this not the same as relative? If it is then should we not call this relative for consistency?

Having said that, if the intention is for relative to be the default then maybe absolute is a better name.

relative, absolute, override. There are many ways of describing this

alteous · 2025-01-06T12:50:36Z

I'll assume the formatting changes in the format specific modules are a result of automated checks.

jtran · 2025-01-06T19:03:15Z

Please add PartialEq and ts-rs derives like in #708 before merging.

benjamaan476 · 2025-01-07T13:29:39Z

I'll assume the formatting changes in the format specific modules are a result of automated checks.

Yeah I don't know why my cargo fmt hits those files. I usually check that before a commit but I must have missed it

modeling-cmds/src/def_enum.rs

Co-authored-by: Adam Chalmers <[email protected]>

benjamaan476 added 2 commits August 8, 2024 15:15

Added commands for object transforming (translate, rotate and scale)

afddc1a

Merge remote-tracking branch 'origin/main' into ben/transform

3804508

benjamaan476 requested review from adamchalmers, lf94 and alteous August 8, 2024 15:02

Change transform commands into one command containing arguments for e…

31376c9

…ach transform type

lf94 approved these changes Aug 8, 2024

View reviewed changes

benjamaan476 and others added 2 commits August 8, 2024 18:17

forgot pub on fields

f618787

Move TransformBy into shared

cf9c069

jtran mentioned this pull request Aug 10, 2024

Cannot move arbitrary objects KittyCAD/modeling-app#2672

Open

benjamaan476 self-assigned this Dec 16, 2024

benjamaan476 added 3 commits December 17, 2024 10:47

merge main

f74a5b2

update modeling api

fb0b02f

Merge remote-tracking branch 'origin/main' into ben/transform

cc90921

alteous reviewed Jan 6, 2025

View reviewed changes

Specify if the transform is to be applied locally or globally

88c7b23

benjamaan476 added 2 commits January 7, 2025 13:45

Turn SetObjectTransform into list of transforms

1d8682e

fix lint issue

876603c

benjamaan476 requested review from alteous and lf94 January 7, 2025 20:39

Rename struct to remove generic

cceca68

benjamaan476 requested review from jtran and gserena01 January 8, 2025 16:10

adamchalmers approved these changes Jan 8, 2025

View reviewed changes

modeling-cmds/src/def_enum.rs Outdated Show resolved Hide resolved

Update modeling-cmds/src/def_enum.rs

ff4a75e

Co-authored-by: Adam Chalmers <[email protected]>

benjamaan476 merged commit d316d5d into main Jan 8, 2025
7 checks passed

benjamaan476 deleted the ben/transform branch January 8, 2025 16:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added commands for object transforming (translate, rotate and scale) #441

Added commands for object transforming (translate, rotate and scale) #441

benjamaan476 commented Aug 8, 2024 •

edited by adamchalmers

Loading

benjamaan476 commented Aug 8, 2024 •

edited

Loading

codecov bot commented Aug 8, 2024 •

edited

Loading

lf94 Aug 8, 2024 •

edited

Loading

adamchalmers Aug 8, 2024

lf94 Aug 8, 2024 •

edited

Loading

benjamaan476 Aug 9, 2024

adamchalmers Dec 13, 2024

benjamaan476 Dec 16, 2024

alteous Jan 6, 2025

alteous Jan 6, 2025

benjamaan476 Jan 7, 2025

alteous commented Jan 6, 2025

jtran commented Jan 6, 2025

benjamaan476 commented Jan 7, 2025

Added commands for object transforming (translate, rotate and scale) #441

Added commands for object transforming (translate, rotate and scale) #441

Conversation

benjamaan476 commented Aug 8, 2024 • edited by adamchalmers Loading

benjamaan476 commented Aug 8, 2024 • edited Loading

codecov bot commented Aug 8, 2024 • edited Loading

Codecov Report

lf94 Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

adamchalmers Aug 8, 2024

Choose a reason for hiding this comment

lf94 Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

benjamaan476 Aug 9, 2024

Choose a reason for hiding this comment

adamchalmers Dec 13, 2024

Choose a reason for hiding this comment

benjamaan476 Dec 16, 2024

Choose a reason for hiding this comment

alteous Jan 6, 2025

Choose a reason for hiding this comment

alteous Jan 6, 2025

Choose a reason for hiding this comment

benjamaan476 Jan 7, 2025

Choose a reason for hiding this comment

alteous commented Jan 6, 2025

jtran commented Jan 6, 2025

benjamaan476 commented Jan 7, 2025

benjamaan476 commented Aug 8, 2024 •

edited by adamchalmers

Loading

benjamaan476 commented Aug 8, 2024 •

edited

Loading

codecov bot commented Aug 8, 2024 •

edited

Loading

lf94 Aug 8, 2024 •

edited

Loading

lf94 Aug 8, 2024 •

edited

Loading