-
Notifications
You must be signed in to change notification settings - Fork 31
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
Image outline #578
base: master
Are you sure you want to change the base?
Image outline #578
Conversation
This is tricky to get right with precise pixel coordinates. Maybe try instead to add a css border to |
If we stick with the current behavior, I would rather keep the ROI creation than moving to css border. It allows the user to easily delete the border like any other ROIs. But... as the PR #549 will create some sync insets, I think it is better to integrate this PR to the The reason I opened this PR was to be able to visually know which zoomed image was linked to which rectangle on the main image. So, if we managed to merge both PRs, it will be very cool ! |
I'm thinking of adding a |
Ok, now I better catch your point. Indeed, it will be easier to include this feature in the right panel. |
I tried experimenting in the browser dev tools to add borders around image panels (see screenshots on #549). I guess we need to decide if the border should go inside or outside the current boundary of the panel? |
Hi, I've modified the code to create a CSS border, as discussed above. The border is set inside the panel, so no need to change panel coordinates. I also have the same issue with the strokewidth drop-down menu that doesn't display well and prevents selecting more than one width. |
A couple of points: Can you toggle the Show/Hide in the same way that the Scalebar does? I don't think we need a separate Remove button. I think we need the border to be outside the regular x,y,w,h rectangle, so that it doesn't affect the image area shown. E.g. duplicated panel, aligned to top: I updated the
|
Conflicting PR. Removed from build OMERO-plugins-push#167. See the console output for more details.
--conflicts |
Ahh - OK, after looking at the PDF export a bit closer, I realise the need for Maybe rename that and the shape type to "border" and add a comment to that method? |
@@ -520,6 +523,24 @@ def __init__(self, pil_img, panel, crop): | |||
self.scale = pil_img.size[0] / crop['width'] | |||
self.draw = ImageDraw.Draw(pil_img) | |||
|
|||
if 'border' in panel and panel['border'].get('showBorder'): | |||
sw = panel['border'].get('strokeWidth') | |||
shift_pos = sw / (float(panel['zoom'])/100) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand the need for diving by the zoom here?
When I was testing on a big image, the zoom was very large, so the shift_pos was effectively zero and the border wasn't getting drawn around the outside of the panel. When I simply tried shift_pos = sw
it seemed to work OK?
With #549 merged, you should be able to merge in master branch now to resolve those conflicts. |
Conflicting PR. Removed from build OMERO-plugins-push#177. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build OMERO-plugins-push#178. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build OMERO-plugins-push#179. See the console output for more details.
|
Hi @will-moore Thanks for the review
Done
This part has been removed to implement the nicer way you proposed for the Tiff export. |
@Rdornier I tried the PDF export and found that it's not handling image rotation very well... If you want to try and reproduce, here's the JSON for that figure, using your image from the Inset PR. If you replace the figure.json
The TIFF export worked better, but still the issue with rotated ROIs. This is possibly a bug from the Inset PR - I'll look into it... One other fix is needed, when the border is a fraction of a
|
Here is the 'rotation' fix which addresses the rotation of 'rectangle' Shapes within panels (not due to this PR). Line numbers apply to
|
Hi @Rdornier - that's nearly there, but the borders just need expanding by 1/2 their thickness so that they are completely outside the edges of the panel. Compare the figure and pdf: |
Hi @will-moore I'm wondering if it is again my dev env which just palying tricks with me. |
Conflicting PR. Removed from build OMERO-plugins-push#208. See the console output for more details.
|
Tested export again. TIFF is fine but PDF the borders are not quite right. I tried tweaking the stroke-width etc but it's getting distorted by the crop and draw_rectangle logic.
This means you could revert other changes such as
|
Hello,
This PR implements the feature from #499.
Almost everything is working properly. I only have one small detail that I couldn't fix. The outline rectangle, even if it copies the viewport coordinates, seem to not be centered on the image. I wasn't able to find a solution yet. Still thinking about it...