Feat/sm layer detection #219
Conversation
|
|
Web tests Screen.Recording.2024-09-10.at.12.10.46.mov |
|
iOS tests Screen.Recording.2024-09-10.at.15.54.07.mov |
feat: cleaned up error catching
chore: fixing git issue with demos chore: demos chore: clippy fix: bindings cpp file fix: bindings cpp file
|
Rust tests Screen.Recording.2024-09-10.at.16.05.46.mov |
samuelOsborne
force-pushed
the
feat/sm-layer-detection
branch
from
6267db1
to
4e3ab58
Compare
dotlottie-rs/src/dotlottie_player.rs
Outdated
| let result = self | ||
| .renderer | ||
| .get_layer_bounds(layer_name) | ||
| .unwrap_or((0.0, 0.0, 0.0, 0.0)); | ||
|
|
||
| vec![result.0, result.1, result.2, result.3] |
There was a problem hiding this comment.
[Suggestion]
We can implement the From trait for LayerBoundingBox, which will allow automatic conversion to a Vec<f32> depending on the execution context.
struct LayerBoundingBox(f32, f32, f32, f32);
impl From<LayerBoundingBox> for Vec<f32> {
fn from(bbox: LayerBoundingBox) -> Vec<f32> {
vec![bbox.0, bbox.1, bbox.2, bbox.3]
}
}
impl Default for LayerBoundingBox {
fn default() -> Self {
LayerBoundingBox(0.0, 0.0, 0.0, 0.0)
}
}Then in get_layer_bounds, we can do something like this:
let bbox = self.renderer.get_layer_bounds(layer_name).unwrap_or_default(); // ensure LayerBoundingBox implements the Default trait
bbox.into() // This will use the conversion mechanism provided by the From traitThere was a problem hiding this comment.
Is there a benefit to adding this? I feel like it makes the code harder to read
|
|
||
| let key = values[0]; | ||
| let numeric_value = values[1].parse::<f32>().map_err(|_| (false)).unwrap(); | ||
| pub fn post_pointer_exit_event(&self, x: f32, y: f32) -> i32 { |
There was a problem hiding this comment.
[question]
This is going to be used to notify the sm that the pointer leaves a certain layer ?
How this would be integrated in dotlottie-web for example ?
Do we need to keep track of the layers we enter and leave in this case ?
| key: key.to_string(), | ||
| value: numeric_value, | ||
| }; | ||
| pub fn post_set_numeric_context(&self, key: &str, value: f32) -> i32 { |
There was a problem hiding this comment.
[questions]
We only have set_numeric_context, Are there any types of context types, like bool, string ..etc ?
| Event::OnComplete => "OnCompleteEvent".to_string(), | ||
| Event::SetNumericContext { key, value } => format!("{}, {}", key, value), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl InternalEvent { | ||
| pub fn as_str(&self) -> String { |
There was a problem hiding this comment.
[question]
Why do we need to convert to String ?
[suggestion]
if there is a need, probably you may want to impl the Display trait for the InternalEvent, as_str makes it as if it returns a String slice, not an owned String as in the curr implementation
| // A layer name was provided, we need to check if the pointer is within the layer | ||
| let pointer_target = target; | ||
|
|
||
| let p = self.player.as_ref().unwrap(); |
There was a problem hiding this comment.
[question]
is it safe to unwrap here ?
| let received_event_values = if let Event::OnPointerDown { x, y } = event { | ||
| Event::OnPointerDown { x: *x, y: *y } | ||
| } else { | ||
| Event::OnPointerDown { x: 0.0, y: 0.0 } |
There was a problem hiding this comment.
[question]
if we are sure that the posted event doesn't match the sm event, why do we return x: 0, y: 0 as a received one ?
| String { value: String }, | ||
| Numeric { value: f32 }, | ||
| OnPointerDown { target: Option<String> }, | ||
| OnPointerUp { target: Option<String> }, |
There was a problem hiding this comment.
[question]
The internal and external pointer event are very confusing, specially in the way they are defined.
for example: OnPointerUp internally has a target of Option but in the external takes x, y as f32.
Maybe can you help me understand the reason for choosing this approach ?
…ottie-thorvg into feat/sm-layer-detection
This PR adds layer hit detection, along with a few refactors
Features:
getLayerBoundsAPI function. Pass a layer name and it returns a vec where the first 4 values correspond to x, y, width, heightperform_hit_check(&self, target: &str, x: f32, y: f32)which returnstrueif the supplied target string (layer name) and x, y coordinate are within its bounding box, otherwise returnsfalseRefactors:
postEventPayloadfor web, replaced with individual functions