Skip to content

Commit

Permalink
Remove storage code
Browse files Browse the repository at this point in the history
The storage code didn't really do anything. It existed as a placeholder.
Or rather, as a tool for API design.

I didn't (and still don't) want to deal with any unnecessary complexity
at this point in the experimentation. But I figured, it would still be
good to write the APIs _as if_ operations were centrally stored. Because
then I wouldn't accidentally design the system in a way, that would make
centrally stored operations impractical.

Because I believed (and still do) that this is going to be necessary for
performance, at some point. (Definitely not right now, at this stage of
the ongoing experiments.) But what I've realized, is that I don't
actually know how exactly this centralized storage needs to be managed.

I just kinda assumed that it makes sense to store operations grouped by
type, as the mainline code currently does. But guess what, that hasn't
been optimized for performance (or even profiled) either.

And since there's currently no code (either in the experiments or
mainline) that iterates over operations by type to mass-update them,
which is the kind of access pattern that would benefit from grouping by
type, I doubt that this was ever a good idea in the first place.

Maybe it would be better to store operations heterogeneously, to
minimize cache misses when iterating over operations as a tree. Maybe it
makes sense to allocate them in multiple arenas, divided into
generations, depending on which are updated when. Maybe.

That's all speculation. But the facts are these: I don't know what the
right approach is going to be, and meanwhile this fake storage code is
slowing down the ongoing experiment. So I've decided to get rid of it.

Of course, this way it's fairly certain that I'll have to re-architect
things, once performance becomes a non-optional requirement. But at that
point I'll also have information on my access patterns, so I'll be in a
position to know what to optimize _for_.

That's going to be a huge pain, of course. But I don't think I can avoid
that, without having the answers to all these questions in advance.

Which I don't.
  • Loading branch information
hannobraun committed Feb 6, 2025
1 parent b23eec5 commit ef73336
Show file tree
Hide file tree
Showing 7 changed files with 12 additions and 87 deletions.
7 changes: 1 addition & 6 deletions experiments/2024-12-09/src/geometry/sketch.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::{
math::{Plane, Point},
storage::Store,
topology::{face::Face, vertex::Vertex},
};

Expand All @@ -11,11 +10,7 @@ pub struct Sketch {
}

impl Sketch {
pub fn to_face(
&self,
surface: Handle<Plane>,
_: &mut Store<Vertex>,
) -> Face {
pub fn to_face(&self, surface: Handle<Plane>) -> Face {
let vertices = self
.points
.iter()
Expand Down
1 change: 0 additions & 1 deletion experiments/2024-12-09/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ mod geometry;
mod math;
mod model;
mod render;
mod storage;
mod topology;
mod view;

Expand Down
12 changes: 2 additions & 10 deletions experiments/2024-12-09/src/model.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
use crate::{
geometry::{AnyOp, Handle, Sketch},
math::{Bivector, Plane, Point, Vector},
storage::Stores,
topology::sweep::SweepExt,
};

pub fn model() -> AnyOp {
let mut stores = Stores::new();

let top = {
let sketch =
Sketch::from([[-0.5, -0.5], [0.5, -0.5], [0.5, 0.5], [-0.5, 0.5]]);
Expand All @@ -20,17 +17,12 @@ pub fn model() -> AnyOp {
},
});

sketch.to_face(surface, &mut stores.vertices)
sketch.to_face(surface)
};

let top = Handle::new(top);

let solid = top.sweep(
[0., 0., -1.],
&mut stores.faces,
&mut stores.surfaces,
&mut stores.vertices,
);
let solid = top.sweep([0., 0., -1.]);

AnyOp::new(solid)
}
35 changes: 0 additions & 35 deletions experiments/2024-12-09/src/storage.rs

This file was deleted.

10 changes: 2 additions & 8 deletions experiments/2024-12-09/src/topology/face.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use spade::Triangulation;
use crate::{
geometry::{AnyOp, Handle, Operation, TriMesh, Triangle},
math::{Plane, Point, Vector},
storage::Store,
};

use super::vertex::Vertex;
Expand Down Expand Up @@ -40,19 +39,14 @@ impl Face {
.map(|(a, b)| [a, b])
}

pub fn flip(&self, _: &mut Store<Plane>) -> Self {
pub fn flip(&self) -> Self {
Self {
surface: Handle::new(self.surface.flip()),
vertices: self.vertices.clone(),
}
}

pub fn translate(
&self,
offset: impl Into<Vector<3>>,
_: &mut Store<Plane>,
_: &mut Store<Vertex>,
) -> Self {
pub fn translate(&self, offset: impl Into<Vector<3>>) -> Self {
let offset = offset.into();

Self {
Expand Down
7 changes: 1 addition & 6 deletions experiments/2024-12-09/src/topology/solid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::fmt;
use crate::{
geometry::{AnyOp, Handle, Operation, TriMesh},
math::Plane,
storage::Store,
};

use super::face::Face;
Expand Down Expand Up @@ -35,11 +34,7 @@ impl Solid {
///
/// It should be seen as more of a placeholder for a real implementation of
/// this operation.
pub fn connect_faces(
[a, b]: [Handle<Face>; 2],
_: &mut Store<Face>,
_: &mut Store<Plane>,
) -> Self {
pub fn connect_faces([a, b]: [Handle<Face>; 2]) -> Self {
assert_eq!(
a.vertices().count(),
b.vertices().count(),
Expand Down
27 changes: 6 additions & 21 deletions experiments/2024-12-09/src/topology/sweep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ use std::fmt;

use crate::{
geometry::{AnyOp, Handle, Operation, TriMesh},
math::{Plane, Vector},
storage::Store,
math::Vector,
};

use super::{face::Face, solid::Solid, vertex::Vertex};
use super::{face::Face, solid::Solid};

pub trait SweepExt {
/// Sweep a face along a path, creating a solid
Expand All @@ -20,29 +19,15 @@ pub trait SweepExt {
///
/// It should be seen as more of a placeholder for a real implementation of
/// this operation.
fn sweep(
self,
path: impl Into<Vector<3>>,
faces: &mut Store<Face>,
surfaces: &mut Store<Plane>,
vertices: &mut Store<Vertex>,
) -> Sweep;
fn sweep(self, path: impl Into<Vector<3>>) -> Sweep;
}

impl SweepExt for Handle<Face> {
fn sweep(
self,
path: impl Into<Vector<3>>,
faces: &mut Store<Face>,
surfaces: &mut Store<Plane>,
vertices: &mut Store<Vertex>,
) -> Sweep {
fn sweep(self, path: impl Into<Vector<3>>) -> Sweep {
let bottom = self;
let top = Handle::new(
bottom.flip(surfaces).translate(path, surfaces, vertices),
);
let top = Handle::new(bottom.flip().translate(path));

let solid = Solid::connect_faces([top, bottom], faces, surfaces);
let solid = Solid::connect_faces([top, bottom]);

Sweep { output: solid }
}
Expand Down

0 comments on commit ef73336

Please sign in to comment.