Skip to content
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.

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

Label on rois #600

Draft
wants to merge 52 commits into
base: master
Choose a base branch
from
Draft

Label on rois #600

wants to merge 52 commits into from

Conversation

Rdornier
Copy link
Contributor

@Rdornier Rdornier commented Nov 8, 2024

Fixes #457

image

The text is created at the same time as the other shapes and they are linked together. It means that

  • Text follows shapes motion
  • Text position is adapted according the shape dimension and stroke width (no manual positionning except the positions within the drop-down menu)
  • If the shape is deleted, the text is also deleted
  • Shapes and text color are the same

This PR is a draft for the moment, because it is not ready yet

  1. It only works for rectangle shapes for now : have to extend to any shapes
  2. The text is not displayed when the modal is closed (but apparently, they are saved, as the number of ROIs displayed in the right panel matches the total number rectangle+text) and it is not display anymore if the modal is open again.
  3. Create automatically text (e.g A,B,C...) when adding insets
  4. The toolbar layout have to be revised (removing add button, fixing the behavior of the text input...)

@will-moore I was wondering if you could help me on point 2 ; I really don't know why shapes don't appear on the corresponding panel. Added to the shape manager, they should be visible. I'm probably missing something but I don't know what. I'll still work on that and keep you posted if I managed to find a solution.
Thanks !

@Rdornier
Copy link
Contributor Author

Ok, I finally found a solution for point 2. I had to rescale the exported coordinates by the zoom_fraction in the json.

@Rdornier
Copy link
Contributor Author

@will-moore I've reached the point where I think it is important for me to get your feedback (I'm not asking for a entire review ; just having a look to see if there is anything obvious I've missed and maybe if the implementation looks correct for you).

Here is a summary of what I've done so far

  • Only one text label is allowed per shape
  • Text labels are created and added to the shapeManager when shapes are created
  • Text label color is the same as the shape stroke color
  • The JSON shape includes the ID of the attached text to be able to find it within the manager.
  • When insets are created, automatic text labels are also created on both ROI and panel
  • When a shape is pasted, the text is replaced by an empty string

For now, everything should work ONLY with rectangle shapes, including the export PDF.
Before extending to other shapes, I would prefer getting your feedback (easiest to modify stuff only for one kind of shape).

I still need to implement (I hope I don't forget anything)

  • shape rotation handling
  • export tiff
  • extend text to all shapes

I'm happy to discuss on anything you think relevant and to take suggestions if you have some.
Thanks.

Rémy.

@will-moore
Copy link
Member

This seems to be mostly working OK for me. I never saw a problem with shape labels showing up in the main figure:

Screenshot 2024-11-29 at 10 18 20

The only bug I've found so far is that the Shape labels aren't getting removed from the Edit ROIs image panel when the dialog is closed and reopened - either with a new panel or with a different panel.

E.g. re-opening the same panel can give an effect like this:

Screenshot 2024-11-29 at 10 25 22

@will-moore
Copy link
Member

Build is failing with:

flake8.main.application   MainProcess    667 INFO     Found a total of 42 violations and reported 4
./omero_figure/scripts/omero/figure_scripts/Figure_To_Pdf.py:376:5: E303 too many blank lines (3)
./omero_figure/scripts/omero/figure_scripts/Figure_To_Pdf.py:391:11: E275 missing whitespace after keyword
./omero_figure/scripts/omero/figure_scripts/Figure_To_Pdf.py:394:13: E275 missing whitespace after keyword
./omero_figure/scripts/omero/figure_scripts/Figure_To_Pdf.py:415:5: E303 too many blank lines (4)

The PDF export has a few issues:

Screenshot 2024-11-29 at 10 45 11

@Rdornier
Copy link
Contributor Author

Rdornier commented Dec 4, 2024

Hi @will-moore,

Thanks for your feedback. I think I've corrected most of the bugs you've reported.
There is only one that I wasn't able to reproduce. When I open/close/reopen the Edit ROI modal dialog, I didn't see the remaining label (I also tried to delete shapes, save and reopen).

Did you do something particular to get this ? If you try again now, does it show the same bug ?
Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

labels on ROI
2 participants