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.

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

Z-offset attribute label #521

Merged
merged 18 commits into from
Nov 30, 2023
Merged

Z-offset attribute label #521

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
dcecb76
Add z-offset option in labels
Rdornier Oct 3, 2023
18e9ee5
Integrate the z-offset in the existing functions and fix bugs
Rdornier Oct 3, 2023
b3061c7
extend offset beyond z-limits
Rdornier Oct 3, 2023
64c7983
Clean code
Rdornier Oct 3, 2023
c48a1ef
fix case where offset is NaN
Rdornier Oct 6, 2023
15df5dc
Add z-offset value to the label
Rdornier Oct 18, 2023
d8e7601
Refactor
Rdornier Oct 18, 2023
7212b38
Add Time offset to label
Rdornier Oct 18, 2023
6bb1381
Remove addings in label; modified the index label accordingly to the …
Rdornier Oct 19, 2023
2190724
Correct bug on positive offset
Rdornier Oct 20, 2023
90522d7
remove offset +1
Rdornier Oct 27, 2023
ffd83e9
Update figure to pdf script
Rdornier Oct 27, 2023
8a28043
Fix offsets
Rdornier Oct 27, 2023
035da8f
Add Z tips
Rdornier Oct 27, 2023
b548f82
coming back to the first behavior for time offset
Rdornier Oct 27, 2023
a35a596
fix description in info panel
Rdornier Nov 14, 2023
68f10d8
Fix typo
Rdornier Nov 21, 2023
f0904f3
Merge branch 'ome:master' into z-offset
Rdornier Nov 22, 2023
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
10 changes: 8 additions & 2 deletions src/js/models/panel_model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@
return text;
},

get_view_label_text: function(property, format, dec_prec) {
get_view_label_text: function(property, format, ref_idx, dec_prec) {
if (format === "px") format = "pixel";

if (property === "w") property = "width";
Expand Down Expand Up @@ -390,10 +390,16 @@
}
else {
var theZ = this.get('theZ');
var deltaZ = theZ;

if (ref_idx) {
var shift = parseInt(ref_idx)
deltaZ = shift == null ? theZ : theZ + shift;
will-moore marked this conversation as resolved.
Show resolved Hide resolved
}
if (format === "pixel") {
text = "" + (theZ + 1);
Copy link
Member

@will-moore will-moore Oct 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I wonder if you could also use deltaZ instead of theZ here, so that you can use offset when showing Z index.
And if you could apply the same change to theT that would be great.
There's maybe not much use case for that (I only noticed it in testing) but that's still the way you'd expect it to behave.
In my test, the white labels (offset=2) show the same as yellow labels (no offset):

Screenshot 2023-10-18 at 09 54 31

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok make sense.
Is it what you had in mind ?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, no! I just meant for that line to be text = "" + (deltaZ + 1); instead of text = "" + (theZ + 1);
That's all! Apologies for the confusion!

Copy link
Contributor Author

@Rdornier Rdornier Oct 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ah...Ok, now I see. No worries, I didn't correctly read.
I've corrected it. I also correct a small bug on time offset (see below) but I noticed that the current implementation doesn't allow for negative offsets for time (but allowed for Z)

                var shift = this.get('deltaT')[shiftIdx];
                deltaT = shift==null ? deltaT : shift;

Do we also allow it for time ?

} else if (format === "unit") {
text = ""+ (theZ * z_size).toFixed(dec_prec) +" "+ z_symbol
text = ""+ (deltaZ * z_size).toFixed(dec_prec) +" "+ z_symbol
}
}
return text
Expand Down
2 changes: 1 addition & 1 deletion src/js/views/panel_view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@
} else if (['x', 'y', 'z', 'width', 'height', 'w', 'h', 'rotation', 'rot'].includes(prop_nf[0])){
format = prop_nf[1] ? prop_nf[1] : "pixel";
precision = param_dict["precision"] !== undefined ? param_dict["precision"] : 2; // decimal places default to 2
label_value = self.model.get_view_label_text(prop_nf[0], format, precision);
label_value = self.model.get_view_label_text(prop_nf[0], format, param_dict["offset"], precision);
} else if (['channels', 'c'].includes(prop_nf[0])) {
label_value = self.model.get_channels_label_text();
}
Expand Down