Skip to content

Overhaul of Voxelization ops with new algorithm. Also included test #657

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

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

Overhaul of Voxelization ops with new algorithm. Also included test #657

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
83b5c4c
Overhaul of Voxelization ops with new algorithm. Also included test
Feb 13, 2026
ff5da48
Remove unused LogService parameter
andmccall Feb 13, 2026
d840532
Minor bug fix
Feb 16, 2026
ff45913
Fix output interval calculation in voxelization
andmccall Feb 16, 2026
b004747
Minor bug fix
Feb 16, 2026
25001a5
Merge remote-tracking branch 'origin/master'
Feb 16, 2026
3b49daf
Minor bug fix
Feb 16, 2026
c3f0c41
Added wallThickness as a parameter, and changed automatically generat…
Feb 17, 2026
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
41 changes: 28 additions & 13 deletions src/main/java/net/imagej/ops/geom/GeomNamespace.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import net.imagej.ops.OpMethod;
import net.imagej.ops.Ops.Geometric.Voxelization;
import net.imagej.ops.geom.geom3d.mesh.VertexInterpolator;
import net.imglib2.Interval;
import net.imglib2.IterableInterval;
import net.imglib2.RandomAccessibleInterval;
import net.imglib2.RealLocalizable;
Expand Down Expand Up @@ -278,13 +279,6 @@ public List convexHull(final Mesh in) {
net.imagej.ops.Ops.Geometric.ConvexHull.class, in);
return result;
}

@OpMethod(op = net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class)
public RandomAccessibleInterval<BitType> voxelization(final Mesh in, final int width, final int height, final int depth ) {
final RandomAccessibleInterval<BitType> result = (RandomAccessibleInterval<BitType>) ops().run(
Voxelization.class, in, width, height, depth );
return result;
}
Comment on lines -281 to -287
Copy link
Copy Markdown
Contributor

@gselzer gselzer Feb 18, 2026

Choose a reason for hiding this comment

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

This is a breaking change 🛠️

Since your Op has no overlap in parameter types with the existing Op, we could consider offering the two Ops side-by-side? Could also consider some sort of deprecation for the old Op...

Copy link
Copy Markdown
Author

@andmccall andmccall Feb 18, 2026

Choose a reason for hiding this comment

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

This would change the algorithm used, so I'm not sure you would want to do this, but you could do:

final RandomAccessibleInterval<BitType> result = (RandomAccessibleInterval<BitType>) ops().run( Voxelization.class, in, new FinalInterval(width, height, depth));

This means that the output from any old code would change, but the function call wouldn't.

I imagine that is not the preferred option, but it doesn't really matter to me, can definitely keep the old algorithm in there in whatever manner you deem best. I simply forgot about the whole "can't make breaking changes" thing, never done any library coding before.

Copy link
Copy Markdown
Contributor

@gselzer gselzer Feb 18, 2026

Choose a reason for hiding this comment

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

This would change the algorithm used, so I'm not sure you would want to do this, but you could do:

final RandomAccessibleInterval<BitType> result = (RandomAccessibleInterval<BitType>) ops().run( Voxelization.class, in, new FinalInterval(width, height, depth));

This means that the output from any old code would change, but the function call wouldn't.

Yes, this option is certainly available, however I think I'd prefer avoiding breakages if possible. I wouldn't say that the previous implementation was a bug, it's just a different algorithm (although certainly a strange one)

I simply forgot about the whole "can't make breaking changes" thing, never done any library coding before.

It happens to the best of us, don't worry 🙂


@OpMethod(op = net.imagej.ops.geom.geom2d.DefaultConvexityPolygon.class)
public DoubleType convexity(final Polygon2D in) {
Expand Down Expand Up @@ -678,26 +672,47 @@ public double[] vertexInterpolator(final int[] p1, final int[] p2,

@OpMethod(op = net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class)
public RandomAccessibleInterval<BitType> voxelization(final Mesh in) {
@SuppressWarnings("unchecked")
final RandomAccessibleInterval<BitType> result =
(RandomAccessibleInterval<BitType>) ops().run(net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, in);
return result;
}

@OpMethod(op = net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class)
public RandomAccessibleInterval<BitType> voxelization(final Mesh in, final int width) {
@SuppressWarnings("unchecked")
public RandomAccessibleInterval<BitType> voxelization(final Mesh in, final Interval dimensions) {
final RandomAccessibleInterval<BitType> result =
(RandomAccessibleInterval<BitType>) ops().run(net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, in, dimensions);
return result;
}

@OpMethod(op = net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class)
public RandomAccessibleInterval<BitType> voxelization(final Mesh in, final Interval dimensions, boolean scaleMeshToDimesions) {
final RandomAccessibleInterval<BitType> result =
(RandomAccessibleInterval<BitType>) ops().run(net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, in, width);
(RandomAccessibleInterval<BitType>) ops().run(net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, in, dimensions, scaleMeshToDimesions);
return result;
}

//Two OpMethods below cause a Mismatched inputs error, though they both work fine if built with 'Skip tests'
// @OpMethod(op = net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class)
// public RandomAccessibleInterval<BitType> voxelization(final Mesh in, double wallThickness) {
// final RandomAccessibleInterval<BitType> result =
// (RandomAccessibleInterval<BitType>) ops().run(net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, in, null, false, wallThickness);
// return result;
// }
//
// @OpMethod(op = net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class)
// public RandomAccessibleInterval<BitType> voxelization(final Mesh in, final Interval dimensions, double wallThickness) {
// final RandomAccessibleInterval<BitType> result =
// (RandomAccessibleInterval<BitType>) ops().run(net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, in, dimensions, false, wallThickness);
// return result;
// }

Comment on lines +694 to +708
Copy link
Copy Markdown
Contributor

@gselzer gselzer Feb 18, 2026

Choose a reason for hiding this comment

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

These do not work because optional Op signatures are only "valid" if parameters are left off right to left. In this case, valid signatures would be:

  • voxelization(final Mesh in, final Interval dimensions, final boolean scaleMeshToDimensions, final double wallThickness)
  • voxelization(final Mesh in, final Interval dimensions, final boolean scaleMeshToDimensions)
  • voxelization(final Mesh in, final Interval dimensions)
  • voxelization(final Mesh in)

Of course these two signatures are not included. The context for this right-to-left dropping is to remove ambiguity when you have two optional parameters that are of the same type e.g. for some

voxelization(final Mesh in, final double a, final Double b)

you could not have both

voxelization(final Mesh in, final double a)
voxelization(final Mesh in, final double b)

However, your use case does not have this problem. You could consider skipping the type check (unclear if this is what you meant by skip tests) - do you think this is an acceptable use case of that parameter @ctrueden?

Suggested change
//Two OpMethods below cause a Mismatched inputs error, though they both work fine if built with 'Skip tests'
// @OpMethod(op = net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class)
// public RandomAccessibleInterval<BitType> voxelization(final Mesh in, double wallThickness) {
// final RandomAccessibleInterval<BitType> result =
// (RandomAccessibleInterval<BitType>) ops().run(net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, in, null, false, wallThickness);
// return result;
// }
//
// @OpMethod(op = net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class)
// public RandomAccessibleInterval<BitType> voxelization(final Mesh in, final Interval dimensions, double wallThickness) {
// final RandomAccessibleInterval<BitType> result =
// (RandomAccessibleInterval<BitType>) ops().run(net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, in, dimensions, false, wallThickness);
// return result;
// }
@OpMethod(op = net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, skipTypeCheck = true)
public RandomAccessibleInterval<BitType> voxelization(final Mesh in, double wallThickness) {
final RandomAccessibleInterval<BitType> result =
(RandomAccessibleInterval<BitType>) ops().run(net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, in, null, false, wallThickness);
return result;
}
@OpMethod(op = net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, skipTypeCheck = true)
public RandomAccessibleInterval<BitType> voxelization(final Mesh in, final Interval dimensions, double wallThickness) {
final RandomAccessibleInterval<BitType> result =
(RandomAccessibleInterval<BitType>) ops().run(net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, in, dimensions, false, wallThickness);
return result;
}

Copy link
Copy Markdown
Contributor

@gselzer gselzer Feb 18, 2026

Choose a reason for hiding this comment

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

Would be great to either get these working or delete them from the code

Copy link
Copy Markdown
Author

@andmccall andmccall Feb 18, 2026

Choose a reason for hiding this comment

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

I'm fine with it either way. Wasn't strongly invested in having these signatures, I just wasn't really sure what the issue was, or how to do the SkipTypeCheck.

Copy link
Copy Markdown
Contributor

@gselzer gselzer Feb 18, 2026

Choose a reason for hiding this comment

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

I also do not feel strongly - I just want to make sure that you get the code you want in, and it's easier to add these things in later than to remove them now. So if you don't have a use let's just remove them?

Copy link
Copy Markdown
Author

@andmccall andmccall Feb 18, 2026

Choose a reason for hiding this comment

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

Sounds good to me.

@OpMethod(op = net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class)
public RandomAccessibleInterval<BitType> voxelization(final Mesh in, final int width, final int height) {
public RandomAccessibleInterval<BitType> voxelization(final Mesh in, final Interval dimensions, boolean scaleMeshToDimesions, double wallThickness) {
final RandomAccessibleInterval<BitType> result =
(RandomAccessibleInterval<BitType>) ops().run(net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, in, width, height);
(RandomAccessibleInterval<BitType>) ops().run(net.imagej.ops.geom.geom3d.DefaultVoxelization3D.class, in, dimensions, scaleMeshToDimesions, wallThickness);
return result;
}


@OpMethod(op = net.imagej.ops.geom.geom2d.DefaultVerticesCountPolygon.class)
public DoubleType verticesCount(final Polygon2D in) {
Expand Down
Loading