will-moore
commented
4 months ago This is tricky to get right with precise pixel coordinates.
Maybe try instead to add a css border to this.$panel_canvas in the panel_view.js?
Open Rdornier opened 4 months ago
will-moore
commented
4 months ago This is tricky to get right with precise pixel coordinates.
Maybe try instead to add a css border to this.$panel_canvas in the panel_view.js?
Rdornier
commented
3 months ago 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 Inset Feature one and to create a css border (not a ROI anymore) with the same color as the sync rectangle (the strock width can be hardcoded). Having the colored border will help visualizing the "filiation".
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 !
will-moore
commented
3 months ago I'm thinking of adding a Border section the Labels tab, with just a colour-picker and stroke-width picker and show/hide button (like the scalebar but simpler).
This is a bit more work, but it will allow you to add, edit & show/hide a border for multiple selected panels at once, and it will be more obvious than adding this under the Edit ROIs dialog.
Rdornier
commented
3 months ago Ok, now I better catch your point. Indeed, it will be easier to include this feature in the right panel. I'll refactor the code in that direction. Thanks.
will-moore
commented
3 months ago 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? E.g. when you add a border, does it go around the outside of the panel (and reduce the gap between panels) or inside the panel and reduce the image area? I can see arguments for both, but I think going inside is slightly easier because going outside means that we also need to update the x/y/width/height coordinates of the panel on the page, since it will take up more space. That's maybe not too hard but just something to think about...
Rdornier
commented
3 months ago 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. However, when I apply the border, the image is shifted to the bottom right corner. I couldn't find a way to remove this shift.
I also have the same issue with the strokewidth drop-down menu that doesn't display well and prevents selecting more than one width.
will-moore
commented
3 months ago A couple of points: I wonder if we could use the term "border" instead of "outline" everywhere (code and UI)? Maybe I'm just thinking css but to me this feature is a border.
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.
This means we need to tweak the actual x,y,w,h of the panel on the page.
Your panel show_outline() wasn't being called when the panel initially renders (only when the outline changes) so when you re-load the figure you don't see it.
E.g. duplicated panel, aligned to top:
I updated the render_layout() etc... (not looked at the form drop-down behaviour yet)...
diff --git a/src/js/views/panel_view.js b/src/js/views/panel_view.js
index ef692336..8bbeac89 100644
--- a/src/js/views/panel_view.js
+++ b/src/js/views/panel_view.js
@@ -40,7 +40,7 @@
'change:channels change:zoom change:dx change:dy change:width change:height change:rotation change:labels change:theT change:deltaT change:theZ change:deltaZ change:z_projection change:z_start change:z_end',
this.render_labels);
this.listenTo(this.model, 'change:shapes', this.render_shapes);
- this.listenTo(this.model, 'change:outline', this.show_outline);
+ this.listenTo(this.model, 'change:outline', this.render_layout);
// During drag or resize, model isn't updated, but we trigger 'drag'
this.model.on('drag_resize', this.drag_resize, this);
@@ -63,6 +63,12 @@
h = xywh[3];
if (w == this.model.get('width') && h == this.model.get('height')) {
// If we're only dragging - simply update position
+ var outline = this.model.get('outline');
+ if (outline != undefined) {
+ let sw = outline.strokewidth;
+ x = x - sw;
+ y = y - sw;
+ }
this.$el.css({'top': y +'px', 'left': x +'px'});
} else {
this.update_resize(x, y, w, h);
@@ -71,15 +77,6 @@
this.$el.addClass('dragging');
},
- show_outline: function(){
- var outline = this.model.get('outline')
- if(outline != undefined){
- this.$el.css({'border': 'solid ' +outline.strokewidth+'px '+outline.color})
- }else{
- this.$el.css({'border': '', 'outline-offset':''})
- }
- },
-
render_layout: function() {
var x = this.model.get('x'),
y = this.model.get('y'),
@@ -92,15 +89,31 @@
update_resize: function(x, y, w, h) {
+ // If we have a panel border, need to adjust x,y,w,h on the page
+ // but NOT the w & h we use for img_css below.
+ var outline = this.model.get('outline');
+ var page_w = w;
+ var page_h = h;
+ if (outline != undefined) {
+ let sw = outline.strokewidth;
+ this.$el.css({'border': `solid ${sw}px ${outline.color}`})
+ x = x - sw;
+ y = y - sw;
+ page_w = w + (sw * 2);
+ page_h = h + (sw * 2);
+ } else {
+ this.$el.css({'border': '', 'outline-offset':''})
+ }
+
// update layout of panel on the canvas
this.$el.css({'top': y +'px',
'left': x +'px',
- 'width': w +'px',
- 'height': h +'px'});
+ 'width': page_w +'px',
+ 'height': page_h +'px'});
// container needs to be square for rotation to vertical
- $('.left_vlabels', this.$el).css('width', 3 * h + 'px');
- $('.right_vlabels', this.$el).css('width', 3 * h + 'px');
+ $('.left_vlabels', this.$el).css('width', 3 * page_h + 'px');
+ $('.right_vlabels', this.$el).css('width', 3 * page_h + 'px');
// update the img within the panel
snoopycrimecop
commented
2 months ago Conflicting PR. Removed from build OMERO-plugins-push#167. See the console output for more details. Possible conflicts:
--conflicts
snoopycrimecop
commented
2 months ago Conflicting PR. Removed from build OMERO-plugins-push#168. See the console output for more details. Possible conflicts:
--conflicts
snoopycrimecop
commented
2 months ago Conflicting PR. Removed from build OMERO-plugins-push#169. See the console output for more details. Possible conflicts:
--conflicts
Rdornier
commented
2 months ago I fixed the logic of the showBorder and rename panel. I also realize that the figure script has to be modified to add the border. I give it a try for both pdf and tiff.
snoopycrimecop
commented
2 months ago Conflicting PR. Removed from build OMERO-plugins-push#174. See the console output for more details. Possible conflicts:
--conflicts Conflict resolved in build OMERO-plugins-push#175. See the console output for more details.
snoopycrimecop
commented
2 months ago Conflicting PR. Removed from build OMERO-plugins-push#176. See the console output for more details. Possible conflicts:
--conflicts
will-moore
commented
2 months ago I used a different strategy for adding a border for Tiff export: Do it by pasting the panel onto a bigger "frame" canvas, right before pasting onto the Tiff page:
NB: you'll need to remove the other border logic for Tiff, and there's also a def draw_outline(self, shape): that can be removed.
@@ -2287,10 +2266,23 @@ class TiffExport(FigureExport):
# Now at full figure resolution - Good time to add shapes...
crop = self.get_crop_region(panel)
- ShapeToPilExport(pil_img, panel, crop)
+ exporter = ShapeToPilExport(pil_img, panel, crop)
width, height = pil_img.size
- box = (x, y, x + width, y + height)
+
+ # Add border if needed - Rectangle around the whole panel
+ if 'border' in panel and panel['border'].get('showBorder'):
+ sw = panel['border'].get('strokeWidth')
+ border_width = scale_to_export_dpi(sw)
+ border_color = panel['border'].get('color')
+ padding = border_width * 2
+
+ canvas = Image.new("RGB", (width + padding, height + padding), exporter.get_rgb(border_color))
+ canvas.paste(pil_img, (border_width, border_width))
+ pil_img = canvas
+ box = (x - border_width, y - border_width, x + width + border_width, y + height + border_width)
+ else:
+ box = (x, y, x + width, y + height)
self.tiff_figure.paste(pil_img, box)Ahh - OK, after looking at the PDF export a bit closer, I realise the need for def draw_outline() - forgot that those draw methods are looked-up on the fly!
Maybe rename that and the shape type to "border" and add a comment to that method?
will-moore
commented
2 months ago With #549 merged, you should be able to merge in master branch now to resolve those conflicts.
snoopycrimecop
commented
2 months ago Conflicting PR. Removed from build OMERO-plugins-push#177. See the console output for more details. Possible conflicts:
--conflicts
snoopycrimecop
commented
2 months ago Conflicting PR. Removed from build OMERO-plugins-push#178. See the console output for more details. Possible conflicts:
--conflicts
snoopycrimecop
commented
2 months ago Conflicting PR. Removed from build OMERO-plugins-push#179. See the console output for more details. Possible conflicts:
--conflicts Conflict resolved in build OMERO-plugins-push#180. See the console output for more details.
Rdornier
commented
2 months ago Hi @will-moore
Thanks for the review
Maybe rename that and the shape type to "border" and add a comment to that method?
Done
I'm not sure I understand the need for diving by the zoom here?
This part has been removed to implement the nicer way you proposed for the Tiff export. The fact I was dividing by the zoom was necessary for the PDF export to get the border at the right scale. If I didn't divide by the zoom, it looks like this
will-moore
commented
2 months ago @Rdornier I tried the PDF export and found that it's not handling image rotation very well... Here's the PDF (left) alongside the figure. All the images have some rotation, except the black-bordered Inset:
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 imageId values in the JSON for the ID of that image in your server, you should be able to "File > Import from 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 pt (e.g. 0.25 pt in that case) we need to round to int for TIFF export:
+++ b/omero_figure/scripts/omero/figure_scripts/Figure_To_Pdf.py
@@ -2278,7 +2278,7 @@ class TiffExport(FigureExport):
# Add border if needed - Rectangle around the whole panel
if 'border' in panel and panel['border'].get('showBorder'):
sw = panel['border'].get('strokeWidth')
- border_width = scale_to_export_dpi(sw)
+ border_width = int(round(scale_to_export_dpi(sw)))Here is the 'rotation' fix which addresses the rotation of 'rectangle' Shapes within panels (not due to this PR). Line numbers apply to main branch rather than this PR, but hopefully not to hard to find:
+++ b/omero_figure/scripts/omero/figure_scripts/Figure_To_Pdf.py
@@ -216,6 +216,30 @@ class ShapeExport(object):
point[0] * tf['A10'] + point[1] * tf['A11'] + tf['A12'],
] if tf else point
+ @staticmethod
+ def apply_rotation(point, centre, rotation):
+ cx = centre[0]
+ cy = centre[1]
+ x = point[0]
+ y = point[1]
+
+ dx = cx - x
+ dy = cy - y
+ # distance of point from centre of rotation
+ h = sqrt(dx * dx + dy * dy)
+ # and the angle
+ angle1 = atan2(dx, dy)
+
+ # Add the rotation to the angle and calculate new
+ # opposite and adjacent lengths from centre of rotation
+ angle2 = angle1 - radians(rotation)
+ newo = sin(angle2) * h
+ newa = cos(angle2) * h
+ # to give correct x and y within cropped panel
+ x = cx - newo
+ y = cy - newa
+ return x, y
+
def draw_rectangle(self, shape):
# to support rotation/transforms, convert rectangle to a simple
# four point polygon and draw that instead
@@ -227,6 +251,17 @@ class ShapeExport(object):
(shape['x'] + shape['width'], shape['y'] + shape['height']),
(shape['x'], shape['y'] + shape['height']),
]
+
+ if shape.get('rotation' or 0) != 0:
+ rotation = shape.get('rotation')
+ # rotate around centre of rectangle
+ cx = shape['x'] + shape['width'] / 2
+ cy = shape['y'] + shape['height'] / 2
+ points = [
+ self.apply_rotation(point, [cx, cy], rotation)
+ for point in points
+ ]
+
s['points'] = ' '.join(','.join(
map(str, self.apply_transform(t, point))) for point in points)
self.draw_polygon(s)
@@ -633,6 +668,15 @@ class ShapeToPilExport(ShapeExport):
(shape['x'], shape['y'] + shape['height']),
]
p = []
+ if shape.get('rotation' or 0) != 0:
+ rotation = shape.get('rotation')
+ # rotate around centre of rectangle
+ cx = shape['x'] + shape['width'] / 2
+ cy = shape['y'] + shape['height'] / 2
+ points = [
+ self.apply_rotation(point, [cx, cy], rotation)
+ for point in points
+ ]
t = shape.get('transform')
for point in points:
transformed = self.apply_transform(t, point)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:
Rdornier
commented
2 months ago Hi @will-moore I've tried to fix it by removing the division by the zoom factor and mutiplying the strokewidth by 1.5. It works quite good for me with big images. However, for small images, I still have the bug described in https://github.com/ome/omero-figure/pull/578#issuecomment-2337433361.
I'm wondering if it is again my dev env which just palying tricks with me. Can you check using one of the image from this zenodo if you also experience the same bug ?
snoopycrimecop
commented
1 month ago Conflicting PR. Removed from build OMERO-plugins-push#208. See the console output for more details. Possible conflicts:
--conflicts Conflict resolved in build OMERO-plugins-push#209. See the console output for more details.
will-moore
commented
1 month ago 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. Instead I reverted to drawing the border directly onto the page and this is looking good. See what you think:
+++ b/omero_figure/scripts/omero/figure_scripts/Figure_To_Pdf.py
@@ -1177,26 +1177,40 @@ class FigureExport(object):
Add any Shapes
"""
if 'border' in panel and panel['border'].get('showBorder'):
- crop = self.get_crop_region(panel)
- sw = panel['border'].get('strokeWidth')
- shift_pos = 1.5*sw
-
- shape = {}
- shape['strokeColor'] = panel['border'].get('color')
- shape['strokeWidth'] = sw
- shape['x'] = crop['x'] - shift_pos
- shape['y'] = crop['y'] - shift_pos
- shape['width'] = crop['width'] + 2 * shift_pos
- shape['height'] = crop['height'] + 2 * shift_pos
- shape['type'] = "border"
- rotation = panel['rotation']
- if rotation != 0:
- shape['rotation'] = 360 - rotation
-
- if "shapes" not in panel:
- panel['shapes'] = [shape]
- else:
- panel['shapes'].append(shape)
+ stroke_width = panel['border'].get('strokeWidth')
+ r, g, b, a = ShapeExport.get_rgba(panel['border'].get('color'))
+ canvas = self.figure_canvas
+ canvas.setStrokeColorRGB(r, g, b, alpha=a)
+ canvas.setLineWidth(stroke_width)
+
+ # by default, line is drawn in the middle of the path
+ # we want it to be on the outside of the xywh coords
+ shift_pos = stroke_width / 2
+
+ p = canvas.beginPath()
+ x = panel['x'] - shift_pos
+ y = panel['y'] - shift_pos
+ width = panel['width'] + (shift_pos * 2)
+ height = panel['height'] + (shift_pos * 2)
+
+ # Handle page offsets
+ x = x - page['x']
+ y = y - page['y']
+
+ # rectangle around the panel
+ points = [[x, y], [x + width, y], [x + width, y + height], [x, y + height]]
+
+ # flip the y coordinate
+ for point in points:
+ point[1] = self.page_height - point[1]
+
+ # same logic as draw_polygon()
+ p.moveTo(points[0][0], points[0][1])
+ for point in points[1:]:
+ p.lineTo(point[0], point[1])
+ for point in points[0:2]:
+ p.lineTo(point[0], point[1])
+ canvas.drawPath(p, fill=0, stroke=1)
if "shapes" not in panel:This means you could revert other changes such as in_panel_check and:
def draw_border(self, shape):
self.draw_rectangle(shape, False)Looks good for both PDF and TIFF export:
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...