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

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

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

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
afddc1a
Added commands for object transforming (translate, rotate and scale)
benjamaan476 Aug 8, 2024
3804508
Merge remote-tracking branch 'origin/main' into ben/transform
benjamaan476 Aug 8, 2024
31376c9
Change transform commands into one command containing arguments for e…
benjamaan476 Aug 8, 2024
f618787
forgot pub on fields
benjamaan476 Aug 8, 2024
cf9c069
Move TransformBy into shared
adamchalmers Aug 9, 2024
f74a5b2
merge main
benjamaan476 Dec 17, 2024
fb0b02f
update modeling api
benjamaan476 Dec 17, 2024
cc90921
Merge remote-tracking branch 'origin/main' into ben/transform
benjamaan476 Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 37 additions & 0 deletions modeling-cmds/src/def_enum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,43 @@
Clone, Debug, Deserialize, JsonSchema, Serialize, ModelingCmdVariant,
)]
pub struct GetNumObjects;

///Set the parent object of an object
#[derive(
Clone, Debug, Deserialize, JsonSchema, Serialize, ModelingCmdVariantEmpty,

Check warning on line 1067 in modeling-cmds/src/def_enum.rs

View check run for this annotation

Codecov / codecov/patch

modeling-cmds/src/def_enum.rs#L1067 

Added line #L1067 was not covered by tests
)]
pub struct SetObjectParent
{
///The object (child) whose parent will be set
pub object_id: Uuid,
/// Id of the new parent, if null will unset any parent of object_id
pub parent_id: Option<Uuid>
}

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

Check warning on line 1077 in modeling-cmds/src/def_enum.rs

View check run for this annotation

Codecov / codecov/patch

modeling-cmds/src/def_enum.rs#L1077 

Added line #L1077 was not covered by tests
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,

Check warning on line 1088 in modeling-cmds/src/def_enum.rs

View check run for this annotation

Codecov / codecov/patch

modeling-cmds/src/def_enum.rs#L1088 

Added line #L1088 was not covered by tests
)]
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>>,
}
Copy link
Contributor

@lf94 lf94 Aug 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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>>
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@lf94 lf94 Aug 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

}
}

Expand Down
29 changes: 2 additions & 27 deletions modeling-cmds/src/format/fbx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,7 @@
pub mod import {
use super::*;
/// Options for importing FBX.
#[derive(
Clone,
Debug,
Default,
Eq,
Hash,
PartialEq,
Serialize,
Deserialize,
JsonSchema,
Display,
FromStr,

)]
#[derive(Clone, Debug, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr)]

Check warning on line 11 in modeling-cmds/src/format/fbx.rs

View check run for this annotation

Codecov / codecov/patch

modeling-cmds/src/format/fbx.rs#L11 

Added line #L11 was not covered by tests
#[display("")]
#[serde(rename = "FbxImportOptions")]
pub struct Options {}
Expand Down Expand Up @@ -64,19 +51,7 @@

/// Describes the storage format of an FBX file.
#[derive(
Default,
Clone,
Copy,
Debug,
Eq,
Hash,
PartialEq,
Serialize,
Deserialize,
JsonSchema,
Display,
FromStr,

Default, Clone, Copy, Debug, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr,

Check warning on line 54 in modeling-cmds/src/format/fbx.rs

View check run for this annotation

Codecov / codecov/patch

modeling-cmds/src/format/fbx.rs#L54 

Added line #L54 was not covered by tests
)]
#[display(style = "snake_case")]
#[serde(rename = "FbxStorage", rename_all = "snake_case")]
Expand Down
16 changes: 1 addition & 15 deletions modeling-cmds/src/format/gltf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

use parse_display::{Display, FromStr};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
Expand All @@ -8,20 +7,7 @@
use super::*;

/// Options for importing glTF 2.0.
#[derive(
Clone,
Debug,
Default,
Eq,
Hash,
PartialEq,
Serialize,
Deserialize,
JsonSchema,
Display,
FromStr,

)]
#[derive(Clone, Debug, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr)]

Check warning on line 10 in modeling-cmds/src/format/gltf.rs

View check run for this annotation

Codecov / codecov/patch

modeling-cmds/src/format/gltf.rs#L10 

Added line #L10 was not covered by tests
#[display("")]
#[serde(rename = "GltfImportOptions")]
pub struct Options {}
Expand Down
5 changes: 1 addition & 4 deletions modeling-cmds/src/format/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

use parse_display_derive::{Display, FromStr};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -53,9 +52,7 @@
}

/// Input format specifier.
#[derive(
Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr,
)]
#[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr)]

Check warning on line 55 in modeling-cmds/src/format/mod.rs

View check run for this annotation

Codecov / codecov/patch

modeling-cmds/src/format/mod.rs#L55 

Added line #L55 was not covered by tests
#[serde(tag = "type", rename_all = "snake_case")]
#[display(style = "snake_case")]
pub enum InputFormat {
Expand Down
5 changes: 1 addition & 4 deletions modeling-cmds/src/format/obj.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

use parse_display::{Display, FromStr};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
Expand All @@ -10,9 +9,7 @@
use super::*;

/// Options for importing OBJ.
#[derive(
Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr,
)]
#[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr)]

Check warning on line 12 in modeling-cmds/src/format/obj.rs

View check run for this annotation

Codecov / codecov/patch

modeling-cmds/src/format/obj.rs#L12 

Added line #L12 was not covered by tests
#[display("coords: {coords}, units: {units}")]
#[serde(rename = "ObjImportOptions")]
pub struct Options {
Expand Down
5 changes: 1 addition & 4 deletions modeling-cmds/src/format/ply.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

use parse_display::{Display, FromStr};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
Expand All @@ -10,9 +9,7 @@
use super::*;

/// Options for importing PLY.
#[derive(
Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr,
)]
#[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr)]

Check warning on line 12 in modeling-cmds/src/format/ply.rs

View check run for this annotation

Codecov / codecov/patch

modeling-cmds/src/format/ply.rs#L12 

Added line #L12 was not covered by tests
#[display("coords: {coords}, units: {units}")]
#[serde(rename = "PlyImportOptions")]
pub struct Options {
Expand Down
17 changes: 2 additions & 15 deletions modeling-cmds/src/format/sldprt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,12 @@
/// Import functionality.
pub mod import {

use parse_display::{Display, FromStr};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

/// Options for importing SolidWorks parts.
#[derive(
Clone,
Debug,
Default,
Eq,
Hash,
PartialEq,
Serialize,
Deserialize,
JsonSchema,
Display,
FromStr,

)]
#[derive(Clone, Debug, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr)]

Check warning on line 9 in modeling-cmds/src/format/sldprt.rs

View check run for this annotation

Codecov / codecov/patch

modeling-cmds/src/format/sldprt.rs#L9 

Added line #L9 was not covered by tests
#[display("split_closed_faces: {split_closed_faces}")]
#[serde(default, rename = "SldprtImportOptions")]
pub struct Options {
Expand Down
15 changes: 1 addition & 14 deletions modeling-cmds/src/format/step.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

use parse_display::{Display, FromStr};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
Expand All @@ -10,19 +9,7 @@
use super::*;

/// Options for importing STEP format.
#[derive(
Clone,
Debug,
Default,
Eq,
Hash,
PartialEq,
Serialize,
Deserialize,
JsonSchema,
Display,
FromStr,
)]
#[derive(Clone, Debug, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr)]

Check warning on line 12 in modeling-cmds/src/format/step.rs

View check run for this annotation

Codecov / codecov/patch

modeling-cmds/src/format/step.rs#L12 

Added line #L12 was not covered by tests
#[display("split_closed_faces: {split_closed_faces}")]
#[serde(default, rename = "StepImportOptions")]
pub struct Options {
Expand Down
5 changes: 1 addition & 4 deletions modeling-cmds/src/format/stl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

use parse_display::{Display, FromStr};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
Expand All @@ -10,9 +9,7 @@
use super::*;

/// Options for importing STL.
#[derive(
Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr,
)]
#[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, Display, FromStr)]

Check warning on line 12 in modeling-cmds/src/format/stl.rs

View check run for this annotation

Codecov / codecov/patch

modeling-cmds/src/format/stl.rs#L12 

Added line #L12 was not covered by tests
#[display("coords: {coords}, units: {units}")]
#[serde(rename = "StlImportOptions")]
pub struct Options {
Expand Down
1 change: 0 additions & 1 deletion modeling-cmds/src/units.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

use kittycad_unit_conversion_derive::UnitConversion;
use parse_display_derive::{Display, FromStr};
use schemars::JsonSchema;
Expand Down
Loading