When I rotate an ellipse annotation in QuPath, it becomes a 100-vertex polygon. This is unfortunate because it means out of several ellipsoidal annotations, some are EllipseROIs and others are PolygonROIs.
I thought that this seemed odd so I did some digging into the source code. From what I can tell, this is because the underlying Java Topology Suite does not support ellipses, and so the moment an ellipse enters JTS-land it gets converted to a polygon. It also seems like it would be difficult to change this, as one would need to introduce support for Ellipses throughout the entire JTS before it is merged into JTS.
Is this analysis correct? What would be the easiest way to add support for rotated ellipses to QuPath, and how feasible is it? I wouldn’t mind putting in some time to write and submit a patch, if it is feasible to do so.
I don’t have anything too useful to add here as far as coding, just a question or two.
Is it necessary to keep them as ellipses? Or could you also save them as polygons if the goal is only to keep them all the same type?
That was my alternate plan, and what I’m working on now. Thankfully, QuPath has exposed the right functions to allow me to convert an ellipse to a polygon with just a few lines of code. In fact, I guess there’s really not much of a downside to using polygons over ellipses, as they are certainly precise enough for our purposes.
Just for the sake of uniformity on our intake code.
Yes, that’s pretty much correct. Well, QuPath supported ellipses that couldn’t rotate before it used Java Topology Suite… but JTS certainly makes it harder/less appealing to create a rotated ellipse class.
I have sometimes wondered if the EllipseROI class should be rewritten to support rotation. This wouldn’t actually need JTS support, because JTS is not used for the internal ROI representation; all that is necessary is that ROIs can be converted to a JTS form (and usually converted back… but sadly not without loss for ellipses).
Adding a new RotatedEllipseROI class probably wouldn’t be too difficult (ROI is an interface to make it easier to add more), but the hard bits are:
adding a corresponding drawing tool (if required)
tolerating the fact that there is no native JTS representation
The last two of these are the main reasons I’m hesitant about including rotates ellipses. Since ROIs are pretty core within the software, they need to be carefully maintained and we are pretty much stuck with any new ROI type after it has been added.
Since QuPath is still at 0.x.x stage, I tend towards having fewer different kinds of ROI for now and aiming to reduce the complexity of the core as much as possible… and ideally also move away from Java serialization for writing .qpdata files. But this is certainly up for discussion and could be revisited in the future.
Sure, I’d consider any pull request It’s the maintenance rather than writing the code that makes me nervous though – and is the reason I haven’t done it myself earlier. ROIs were completely rewritten for v0.2.0, and it involved quite a lot of complicated gymnastics to try to make sure everything remained compatible to continue supporting annotations drawn in previous versions.
It’s probably cleaner to update the existing EllipseROI than to create an entirely new class. But the lack of JTS support is already a bit problematic, since JTS pretty much sets the standard in this area (even with software not written in Java). The polygonization required to handle ellipses with JTS is a bit troublesome for QuPath’s hierarchical data model, since it can subtly influence when a ROI ‘covers’ or ‘contains’ another ROI – giving different results for the polygonized ellipse compared to what the ‘true’ result would be.
None of this is decisive. However, for those reasons, the most important question for me is why rotated ellipses would be useful. If there isn’t a compelling reason (that outweighs the support costs) then I’d rather not add further complexity to ROIs at this time.
I had a quick look, and it would be possible to retrofit an angle property to ellipse ROIs, but it would require a quite a bit of testing and updating other classes – there are multiple other bits of the code that would need to be updated to make it well-behaved. For example, the RoiEditor (the handles are in the wrong place, so in this hacked-together example the ROI couldn’t be resized).
An entirely new ROI class (that isn’t editable) would probably be easier to integrate… but I’m not sure it would be the right way to go.