Draft: Potential directions for fj-viewer architecture.

[freylint]

Relevant Matrix Conversation
Based on your response I feel its best to first justify an ECS, then justify Bevy, and finally Propose a potentially more fitting alternative which incorporates my disagreements with the bevy project.

Note: Due to the length of this I've inserted anchors into this post. Please quote (or anchors in matrix) when referring to a specific section.

Table of Contents

• Where We Are
• Directions and Requirements
  • The Current Direction

Where We Are

The way I conceptualize the current implementation is that it functions as a very low level implementation of the actor models. Its quite elegant at this scale, but I worry that it will run into ownership issues as fj-viewer is scaled or forked to build a hypothetical editor.
I quite like the efficiency of the current implementation, but I think it won't scale if fornjot wants to scale past a model viewer.
The issue I have with this is covered in more detail in Catherine West's blogpost on the benefits of ECS in gamedev or its associated talk. I highly suggest you are familiarize yourself with these before participating in this discussion.

Directions and requirements

I see four architectures we could go with

• Continuing the current architecture
• Higher level Actor model ie: messages + smol
• High level ecs ie: bevy_ecs / hecs
• Custom Low Level ECS: An ecs built for our specific needs with minimal runtime

The Current Direction

My issues with the current architecture are more or less already covered in Catherine's talk, but I'd like to go through an example in the actual project that demonstrates my concern for continuing in this direction, the interaction between input and camera rotation.

First lets take a look at the start of this interaction, the (input)Handler struct:

/// Input handling abstraction.
///
/// Takes user input and applies them to application state.
pub struct Handler {
    cursor: Option<PhysicalPosition<f64>>,

    movement: Movement,
    rotation: Rotation,
    zoom: Zoom,
}

So viewing this struct definition in isolation, I parse it as something like:

struct InputHandler {
    cursor_pos: CursorState,
    movement: AnyMovement,
    rotation: AnyRotation,
    zoom: CameraZoom,
}

where movement, rotation, and zoom are caches for messages to be sent to the camera.
That seems good so far. How about we look at how this data gets used?

/// Applies cursor movement to `camera`.
pub fn handle_cursor_moved(
    &mut self,
    cursor: PhysicalPosition<f64>,
    camera: &mut Camera,
    window: &Window,
) {
    if let Some(previous) = self.cursor {
        let diff_x = cursor.x - previous.x;
        let diff_y = cursor.y - previous.y;

        self.movement.apply(self.cursor, camera, window);
        self.rotation.apply(diff_x, diff_y, camera);
    }

    self.cursor = Some(cursor);
}

Allright, so after reading this I can update my parsing of (input)Handler.

struct InputHandler {
  cursor_pos: CursorState,
  movement: CameraMovementRequest,
  rotation: CameraRotationRequest,
  zoom: CameraZoom,
}

And due to the mutation of Camera in the function signature

camera: &mut Camera,

I start to conceptualize this as a function that updates the camera position and rotation by sending a mutation request via the CameraMovementRequest message structure.

so weve got a data flow like: User Input -> InputHandler -> MovementRequest -> Movement(Camera)
Which in this high level representation of what the code is doing is fantastic!
Let's take a look to see what the code is actually doing though.

let diff_x = cursor.x - previous.x;
let diff_y = cursor.y - previous.y;

self.movement.apply(self.cursor, camera, window);
self.rotation.apply(diff_x, diff_y, camera);

So I'm reading this as applying a movement to camera by passing a mutable reference to camera to the movement type?
This seems super confusing by itself so lets look at the function we're calling here.

pub fn apply(
    &mut self,
    cursor: Option<PhysicalPosition<f64>>,
    camera: &mut Camera,
    window: &Window,
) {
    if let (Some(previous), Some(cursor)) = (self.cursor, cursor) {
        let previous = camera.cursor_to_model_space(previous, window);
        let cursor = camera.cursor_to_model_space(cursor, window);

        if let Some(focus_point) = self.focus_point.0 {
            let d1 = distance(&camera.position(), &cursor);
            let d2 = distance(&camera.position(), &focus_point);

            let diff = (cursor - previous) * d2 / d1;
            let offset = camera.camera_to_model().transform_vector(&diff);

            camera.translation.x += offset.x;
            camera.translation.y += offset.y;
        }
    }

    self.cursor = cursor;
}

Okay so the Movement type is directly mutating the camera state.
This is a functional, and when I'm not being unfair for analytical purposes, quite understandable function, but I think there's room for improvement.
This seems to be modelling the behavior of the actor model using low level rust.
Which admittedly is quite an awesome achievement.
It does, however, make me question if the code would be easier to understand and maintain if we stepped up a level of abstraction and formally adopted the actor model.

Switching to a more formal actor model

Using an existing ECS

Using a bespoke ECS

[hannobraun]

Hey @freylint! First off, thank you for starting this discussion!

I'd like to emphasize that constructive criticism is always welcome, and if your analysis is helping you improve your understanding of what needs to be done, then keep going! However, I have to say that from my perspective, critiquing the current implementation seems a bit beside the point. What's important, in my opinion, is what purpose that implementation achieves, and what else we want it to achieve going forward.

What the code does specifically to achieve that purpose is secondary, an implementation detail. And not a very deliberate implementation detail at that. It's just how it ended up, given the goals I had when writing it, and the constraints I was working under. It would look very differently, if someone else had written it, or if I had put more effort into it, instead of focusing on the kernel side. So you don't need to convince me that it has to change. Of course does, we want it do more interesting things 😁

I would suggest we go about it like this:

1. Catalogue what the code currently does.
2. Collect the additional things we want it to do
3. Determine what kind of architecture might best serve this purpose. (Emphasis on might, because the actual implementation is going to provide more insights that could and should change that plan.)
4. Figure out how we want to get there. Incremental changes? Or a total rewrite?
5. Do it!

As for step 1., cataloguing what the code currently does, here's my first attempt:

1. Display a triangle mesh with some shading.
2. Display limited debug data. This is of limited use right, and could be scrapped in a new version, as long as there's a realistic path for re-adding that kind of thing later.
3. Non-trivial approach to camera control that doesn't work correctly (Computation of focus point is not fully correct #18, Zooming is way too slow for large models #25, Rotation should still work, if mouse is not hovering over model #140). Could be scrapped and replaced with something simpler.
4. Simple UI interaction; right now just to enable/disable displaying of debug data, and show the current status.
5. Portable to all major desktop and mobile platforms, as well as browsers. That is not fully realized, but the current tech stack enables such ports. I think it's critical to preserve this potential.

This is a short list, and a lot of it doesn't pull its own weight (like the non-trivial and non-working camera control). Which is why I think that a total rewrite that doesn't preserve every detail of the current implementation could be an option.

As for step 2., what I'd like to see in the future, here's a partial list (partial, because I'm sure we can come up with a lot more, given time):

• Rendering improvements:
  • Render edges #16
  • potentially Show focus point #17
  • maybe even Experiment with displaying background #21
• There should be infrastructure to make it easy to add new UI elements. I've had my eye on egui to achieve this, but no decision has been made.
• New UI elements:
  • Display model structure #13, Fine-grained control over debug output #241
  • Add UI element that displays orientation of model #15
  • Display model parameters as modifiable UI elements #35
  • Some kind of error log (Handle errors in a user-friendly way #68)

As for steps 3. and beyond, if I were to do this on my own, I would probably keep extending the current fj-viewer incrementally, adding egui or something comparable, while keeping an eye out for high-level rendering engines that could replace all that wgpu code. While I think that your criticisms of the input architecture are on point, I also think that's the least of fj-viewer's problems.

But as I've stated before, I'm not going to work on this any time soon. Since you seem willing to do it, I'm happy to give you a lot of leeway. If you think Bevy is the right way forward (just using it as an example, I know you've expressed doubts about that), I'm happy to go with that, even though I would have done it differently.

---

Okay, enough rambling. I'd like to address a few specific points from your post.

The way I conceptualize the current implementation is that it functions as a very low level implementation of the actor models.

That's interesting! I never though about it in this way, but I do think it's on point.

Higher level Actor model ie: messages + smol

Two comments about that:

1. I do worry that going with a formal message-passing implementation sounds a bit like over-engineering right now. Personally, I'd err on the side of keeping it simple, while leaving the option to switch to a formal actor model open.
2. One goal of Fornjot is web support, and my understanding is that smol doesn't run in the browser.