forked from napari/napari
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Scale bar #6
Open
melonora
wants to merge
116
commits into
main
Choose a base branch
from
scale_bar
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
…ora/napari into screenshot_without_margins
Co-Authored-By: [email protected]
Co-authored-by: olusesan [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
Co-authored-by: [email protected]
# References and relevant issues closes napari#7382 # Description We don't actually need big data because we monkeypatch tifffile's writer to raise the bigtiff data error even on small arrays. Additionally, we can use np.empty rather than np.random.random because we don't care about the array contents. --------- Co-authored-by: Juan Nunez-Iglesias <[email protected]>
# References and relevant issues `n_dimensional` was removed in napari#3377 # Description I came across a reference to `n_dimensional` in the docs and realized it didn't exist. In this PR I remove it from the test and example. Here's the docs PR where I also switch to the proper property: napari/docs#522 Co-authored-by: Draga Doncila Pop <[email protected]>
# References and relevant issues Refactoring keybindings. # Description This PR uses the pytest mark functionality to identify tests that are related to keybindings. As things with keybindings are refactored, this would speed up local testing of keybinding functionality. To run the keybinding tests, enter: ```sh pytest -v -m key_bindings ``` cc/ @lucyleeow You may find this useful. If so, I will take this out of draft.
Updated packages: `contourpy`, `coverage`, `dask`, `fonttools`, `hypothesis`, `lxml-html-clean`, `numcodecs`, `tensorstore`, `tomli` Co-authored-by: napari-bot <[email protected]>
# References and relevant issues Closes: napari#7074 # Description Instead of updating the highlight on every draw, just do it when the mouse moves or press.
…7381) # References and relevant issues Addresses napari#7363 # Description This PR updates the docstrings and comments to help clarify behavior of the dedicated status checker thread and its relationship with the viewer. Add a bit more context about the signal between the viewer and the thread.
# Description The current examples `to_screenshot.py` and `export_figure.py` are not immediately clear on the differences and limitations of each. I have combined each document into one new example, with added comparisons to clearly show how margins are handled. Furthermore, this example clarifies that overlays like `scale_bar` are also exported with the figure and dependent on the margins, which is not immediately obvious since a canvas view can have the `scale_bar` outside the margins of the data. This example clearly shows that the scale bar is moved to fit within the extent of the data. I believe that combining the examples in this way will improve user understanding of these two features, and make searching for a screenshot without margins feature easier. I would also be interested in removing both `to_screenshot.py` and `export_figure.py` since this example contains clarity on functionality of both, such as the scale bar, that is not present in the prior files (though this could be added). Also note that the original docstrings for both files are incorrect and should otherwise be updated. Finally, this example shows how the scale bar length is dynamically adjusted with calls to `export_figure` which may be confusing for users. I'm not sure if adding `viewer.scale_bar.length = 150` would make this functionality more or less clear to the user, so I've added it as a comment for now. ![image](https://github.com/user-attachments/assets/73f6f881-986a-4811-b903-54739a23598f) --------- Co-authored-by: Peter Sobolewski <[email protected]> Co-authored-by: Grzegorz Bokota <[email protected]>
…s and change controls order (plane controls under depiction combobox) (napari#7395) # References and relevant issues Closes napari#7393 # Description A preview: ![depiction_plane](https://github.com/user-attachments/assets/c8876f75-7e52-4dff-a707-acda593d859c)
updates: - [github.com/astral-sh/ruff-pre-commit: v0.7.3 → v0.7.4](astral-sh/ruff-pre-commit@v0.7.3...v0.7.4) --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Grzegorz Bokota <[email protected]>
) # References and relevant issues Closes napari#7067 # Description Allow to do mouse pan and zoom when there is no active layer (no layer is selected or multiple layers are being selected). More info on the definition of an active layer: > There can only be one ``active`` and one ``current`` item, but there can be > multiple selected items. An "active" item is defined as a single selected > item (if multiple items are selected, there is no active item). The > "current" item is mostly useful for (e.g.) keyboard actions: even with > multiple items selected, you may only have one current item, and keyboard > events (like up and down) can modify that current item. It's possible to > have a current item without an active item, but an active item will always > be the current item. from: https://github.com/napari/napari/blob/195bbd0720fce1bae665cd18ccee5456a095b830/napari/utils/events/containers/_selection.py#L25-L32 A preview: ![non_active_pan_zoom](https://github.com/user-attachments/assets/98367a59-8658-4b1e-b9db-db80ffb5ee63)
…i#6699) # References and relevant issues Closes napari#1411 # Description Add a test for the layer list drag and drop behavior (use `pyautogui` and the `skip_local_focus` decorator): ![dragdrop](https://github.com/napari/napari/assets/16781833/3d62316d-9332-43ba-a252-1eb9a70fd88b) --------- Co-authored-by: Peter Sobolewski <[email protected]>
# References and relevant issues Closes napari#7409 # Description In this PR the `Return` keybind is consumed by the LayerList and not passed up to the viewer. This is the same approach as a previous fix napari#4259 which did the same for Delete/Backspace.
# References and relevant issues Closes napari#7415 # Description Add metadata event in Layer EmitterGroup ### Example ```python from skimage import data import napari # create the viewer with an image viewer = napari.view_image(data.astronaut(), rgb=True) def observer(event): print(event.source.metadata) viewer.layers[0].events.metadata.connect(observer) viewer.layers[0].metadata = {'name': 'astronaut'} # prints {'name': 'astronaut'} viewer.layers[0].metadata['name'] = 'astro' # does not print anything if __name__ == '__main__': napari.run() ``` Note that the setter is not triggered when you manipulate directly the underlying dictionary object. I don't know if this is something we want?
# References and relevant issues Partially addresses napari#7382 # Description This PR marks the 10 slowest tests on my local machine. This allows me to include or exclude when I run tests locally. This could be an initial proof of concept step that could be used to identify some slow tests on CI runs and treat them differently than the rest of the test suite. Perhaps exclude on some test matrix runs or parallelize the slow tests. --------- Co-authored-by: Peter Sobolewski <[email protected]> Co-authored-by: Grzegorz Bokota <[email protected]> Co-authored-by: Juan Nunez-Iglesias <[email protected]>
updates: - [github.com/astral-sh/ruff-pre-commit: v0.8.0 → v0.8.1](astral-sh/ruff-pre-commit@v0.8.0...v0.8.1) - [github.com/python-jsonschema/check-jsonschema: 0.29.4 → 0.30.0](python-jsonschema/check-jsonschema@0.29.4...0.30.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…ari#7418) # References and relevant issues Closes napari#7314 Closes napari#7348 (supersedes) # Description Change `mode` following `visible` attribute value (changing `mode` to be pan/zoom if the layer is not visible and saving the previous mode to restore it when the layer is visible again) and prevent changing `mode` to something different to pan/zoom while the layer is not visible: ![label_mode_visible](https://github.com/user-attachments/assets/13736aa5-c06d-45c6-9823-623a61417c3c)
# References and relevant issues Addresses / Closes? napari#7425 # Description Examples using `screenshot` and `figure_export` were found to have a grayscale canvas in the docs. The grayscale appears to be from the `flash=True` argument, so this PR sets `flash=False` to fix the display of the viewer for the Gallery. The canvas will now properly be color. This does not fix the root cause of the grayscale canvas as a result of `flash=True`. Tested with a local docs build (finally!) Thank you @psobolewskiPhD for assistance.
…ally (napari#7432) # References and relevant issues Closes napari#7394 # Description A preview: ![surface_shading](https://github.com/user-attachments/assets/3a89699d-da06-4aba-bccd-fe318a8ed74d) --------- Co-authored-by: Grzegorz Bokota <[email protected]>
…#7422) # References and relevant issues Closes napari#7008 # Description This implements a world to data transformation for normals, which is needed by the render plane implementation. It's an adaptation of code from napari-threedee to nD by extracting a submatrix from the layer transform matrix and using that. The OP of napari#7008 has extra details.
) # References and relevant issues Closes : napari#7434 # Description Juan merged napari#7422 while I was working on it! This PR adds a simple 3D and 4D test of layers with non-uniform scale (transform) and computing the normal using displayed dims. These tests failed before napari#7422 (using ray not normal transform) but should pass now with the new method.
…#7426) # References and relevant issues Closes napari#7405 # Description Updates docstrings for `screenshot` and `export_figure` examples to be descriptive. Replaces previous incorrect (perhaps placeholder) text. --------- Co-authored-by: Juan Nunez-Iglesias <[email protected]> Co-authored-by: Peter Sobolewski <[email protected]>
# Description I think it was too optimistic to add myself to `qt/`. Happy to be pinged on app-model/action related Qt stuff though.
updates: - [github.com/astral-sh/ruff-pre-commit: v0.8.1 → v0.8.2](astral-sh/ruff-pre-commit@v0.8.1...v0.8.2) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
# References and relevant issues From discussion on image.sc forum: https://forum.image.sc/t/napari-labels-layers-borders-displays-with-weird-shading/105499/1 # Description This modifies the 3D isosurface shader for the Labels layer in a few ways. The goal is to improve appearance of the 3D rendered labels. Specifically it should be improved at the boundaries of the volume (for both `fast` and `smooth` gradient modes) and for 1px thin labels (for `smooth` gradient mode). Changes include: * A new uniform to toggle what happens when sampling _outside_ the volume: * clamping to the edge (default, current behavior) * returning the background value * Update code to prevent dark surfaces in very thin (1px) labels * Add check for adjacent background pixels to the `smooth` gradient function * Update the gradient calculation to include an estimate of the local gradient instead of setting a default lambert term The configurable clamping behavior has the effect of giving labels "dark" (almost hollow-looking) or "filled" faces at the borders of the label volume. Currently this is controlled by a property on the `Volume` visual, but toggling is not exposed in the UI. I think at least @jni is requesting to be able to toggle this in the UI. In the animation below this is what the floating magicgui widget is toggling. I'm happy to wire that up in this PR if there's agreement, but I could use input on a name. We also need to choose a default. ![labels-edges-pr](https://github.com/user-attachments/assets/728505be-d212-417b-a29e-7228761ffed3) I did a quick benchmark test on my machine and did not see significant performance hit. I will try to do a longer test as well. Finally, I need to look a bit into what the "background value" is. Right now it's hard-coded in the shader, but it sounds like perhaps the `Labels` layer may be able to have a non-zero background. I want to make sure to support that or at least not cause a regression.
# References and relevant issues # Description Similar to napari#7256 this PR is a speedup of face triangulation by avoiding a lot of reallocation of arrays. After long investigation, I found that using best way to speed this up is by avoiding the allocation of additional arrays by using numba. For shapes that do not have bevels: On main: ![obraz](https://github.com/user-attachments/assets/b6775d94-a88b-4a99-959f-f4b6f7f768aa) In this PR: ![obraz](https://github.com/user-attachments/assets/9a3a302c-2eb1-4ecd-8c38-a07617512ede) PR with cache: ![obraz](https://github.com/user-attachments/assets/6d7f00a8-4d55-4bb2-9f49-955bc746e01f) As you may see, the time of `triangulate_edge` is reduced from 44s to 17s. Half of it is numba compilation. With sharp edges that require bevel join, the speedup is even bigger: Main: ![obraz](https://github.com/user-attachments/assets/e2a55f13-6ba7-4f1d-9ff9-56fa46fdf49c) PR: ![obraz](https://github.com/user-attachments/assets/6279350e-1e50-441e-a538-4f01b5420d40) PR with cache: ![obraz](https://github.com/user-attachments/assets/3c992928-b63c-42d7-be6d-d0ec7ec620f5) This PR also fixes the bevel join On main: ![obraz](https://github.com/user-attachments/assets/9a24bc31-9777-4d60-81d5-ff6cc1b0d1a2) With PR: ![obraz](https://github.com/user-attachments/assets/b167a4d5-9bce-442c-80c6-d5a0c90e5f8d) In this PR I have added `examples/dev/triangle_edge.py` with a set of helper functions to visualize triangulation. These are helpful for debugging of triangulation problems. # Algorithm explanation Here I add some explanation to the code implementation of this PR. ## Calculation of miter vector <a href="#user-content-miter" id="miter">#</a> This is a screenshot of miter join: ![obraz](https://github.com/user-attachments/assets/08d2dd78-bb02-4da8-baa5-7b0596e1c3c7) And let's assume markings as on image: ![obraz](https://github.com/user-attachments/assets/d0cec9ff-3786-4edc-805a-d45e58f33aab) The $\vec{BA}$ (blue) is half of the normal vector from vertex $n$ to $n+1$. The $\vec{AC}$ is minus half of the normal vector from vertex $n-1$ to $n$. The vector $\vec{AD}$ (green) is orthogonal to line $BA$ of length $0.5$ Based on these vectors, we could easily calculate $\vec{BC} = \vec{BA} + \vec{AC}$. However, the length of $|GC|$ may be different from 1. We would like to get the vector $\vec{BE}$ We will use here trigonometry and [Intercept theorem](https://en.wikipedia.org/wiki/Intercept_theorem) We know that $\frac{|GC|}{|AC|} = \sin{\alpha}$ and $|AC| = \frac{1}{2}$ so $|GC| = \frac{\sin{\alpha}}{2}$ From intercept theorem, we know that $\frac{|BC|}{|GC|} = \frac{|BE|}{|FE|}$. The length of $|FE| = \frac{1}{2}$. So $|BE| = |BC| \times \frac{1}{\sin{\alpha}}$. That mens that $\vec{BE} = \vec{BC} \times \frac{1}{\sin{\alpha}}$ ## Bevel limit <a href="#user-content-bevel-limit" id="bevel-limit">#</a> In general, we use miter join, however, if the angle is sharp, it may result in strange artifacts (see napari#6706). So if the join will be too long (The length of $\vec{BA}$ vector from above), we would like to change to a bevel join. So we need to calculate the length of $\vec{BE}$. We would like to avoid calculation of square root in every turn of the loop. From Pythagoras theorem we know that $|BE|^2 = |EF|^2 + |BF|^2 = \frac{1}{2}^2 + |BF|^2$ We also know that $|GA| = \frac{\cos{\alpha}}{2}$ so $|BG| = \frac{1 - \cos{\alpha}}{2}$. From miter vector calculation and intercept theorem we know that $|BF| = \frac{|BG|}{\sin{\alpha}} = \frac{1 - \cos{\alpha}}{2\sin{\alpha}}$ So we know the length of the vector $$|BE|^2 = \frac{1}{2}^2 + \left(\frac{1 - \cos{\alpha}}{2\sin{\alpha}}\right)^2 = \frac{\sin^2{\alpha} + 1^2 - 2\cos{\alpha} + \cos^2{\alpha}}{4\sin^2{\alpha}} = \frac{2 - 2 \cos{\alpha}}{4\sin^2{\alpha}} $$ Using trigonometric identity $$|BE|^2 = \frac{1}{2} \cdot \frac{1 - \cos{\alpha}}{1 - \cos^2{\alpha}} = \frac{1}{2} \cdot \frac{1 - \cos{\alpha}}{(1 - \cos{\alpha})(1 + \cos{\alpha})} = \frac{1}{2} \cdot \frac{1}{1+\cos{\alpha}}$$ So if we would like to check condition $|BE| > limit$ we could use the following equation: $$ \frac{1}{2} \cdot \frac{1}{1+\cos{\alpha}} > {limit}^2$$ So: $$ \frac{1}{2 \cdot {limit}^2} - 1 > \cos{\alpha} $$ It means that if $\cos\alpha$ is below a given threshold, we should use bevel join. ## Too long vector in bevel join <a href="#user-content-bevel-cut" id="bevel-cut">#</a> As I showed above, this PR is fixing the bevel join by fixing the position of inner edge of the bevel join. You may see an example in this image (see the longest yellow vector): ![obraz](https://github.com/user-attachments/assets/58e13821-a1b6-4dd0-a49b-ade97edf9e73) The schematic of generating long edge: ![obraz](https://github.com/user-attachments/assets/329b2b61-3c9d-4050-9623-7053c1f46248) As users may use shapes with different length edges, we cannot use a hard-coded limit for the length of the offset vector that creates the problems pointed on above image. However, when calculating the normal vectors, we need to calculate the edge width, so we may save them for later usage and limit the length of such problematic offset vector to length of shortest adjusted edge. In the Bevel limit section, we have an explicit formula for miter offset length. However, it contains a square root. And calculation of a square root is expensive on current CPU, so we would like to avoid it. However, the described problem happens only when $\measuredangle BAC$ is close to $180\degree$, but in such a situation, the $|\vec{BA} + \vec{AC}|$ is close to 1 (as resulted vector is close to diameter). So we could approximate the length of vector $\vec{BC}$ as 1. Then the estimated length of vector $\vec{BE}$ is $\frac{1}{\sin{\alpha}}$ So if $\frac{1}{\sin{\alpha}}$ is bigger than the length of half of any adjusted edges, we could calculate a new scaling factor with value $$\pm 0.5 \cdot\min(vec1\textunderscore{}len, vec2\textunderscore{}len))$$ # Additional notes ## Artifacts There are still artifacts in rendering ![obraz](https://github.com/user-attachments/assets/416df018-f293-425c-bd38-916ba8137934) --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Carol Willing <[email protected]> Co-authored-by: Juan Nunez-Iglesias <[email protected]>
…geio`, `ipython`, `matplotlib`, `napari-console`, `pydantic`, `pyqt6`, `pytest`, `scikit-image`, `superqt`, `tensorstore`, `tifffile`, `tqdm`, `virtualenv`, `xarray`, `zarr` (napari#7406) Updated packages: `app-model`, `asttokens`, `attrs`, `certifi`, `coverage`, `dask`, `debugpy`, `fonttools`, `hypothesis`, `imageio`, `ipython`, `matplotlib`, `napari-console`, `numcodecs`, `pydantic`, `pydantic-core`, `pyqt5-sip`, `pyqt6`, `pyqt6-qt6`, `pyqt6-sip`, `pytest`, `rpds-py`, `scikit-image`, `six`, `superqt`, `tensorstore`, `tifffile`, `tomli`, `tornado`, `tqdm`, `typer`, `virtualenv`, `xarray`, `zarr` --------- Co-authored-by: napari-bot <[email protected]> Co-authored-by: Grzegorz Bokota <[email protected]>
updates: - [github.com/astral-sh/ruff-pre-commit: v0.8.2 → v0.8.3](astral-sh/ruff-pre-commit@v0.8.2...v0.8.3) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References and relevant issues
Description