Skip to content

Commit

Permalink
apply review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sedghi committed Aug 30, 2023
1 parent 494e2fd commit 50d0d1e
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,7 @@ export default function toggleStackImageSync({
displaySetInstanceUID
);

if (displaySet?.isReconstructable) {
return true;
}

return false;
return !!displaySet?.isReconstructable;
}
});

Expand All @@ -68,9 +64,8 @@ export default function toggleStackImageSync({
}

const { element } = cornerstoneViewportService.getViewportInfo(viewportId);
const { viewport: csViewport, renderingEngineId } = getEnabledElement(
element
);
const { viewport: csViewport, renderingEngineId } =
getEnabledElement(element);
const { viewPlaneNormal } = csViewport.getCamera();

// Should we round here? I guess so, but not sure how much precision we need
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
"postcss-import": "^14.0.2",
"postcss-loader": "^6.1.1",
"postcss-preset-env": "^7.4.3",
"prettier": "^1.18.2",
"prettier": "^3.0.3",
"react-hot-loader": "^4.13.0",
"semver": "^7.5.1",
"serve": "^14.2.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ const match = (
errorMessages = ['Something went wrong during validation.', e];
}

// TODO: move to a logger
// console.log(
// 'Test',
// `${from}.${attribute}`,
// readValues[attribute],
// JSON.stringify(rule.constraint),
// !errorMessages
// );

if (!errorMessages) {
// If no errorMessages were returned, then validation passed.

Expand Down
46 changes: 39 additions & 7 deletions platform/docs/docs/platform/extensions/modules/viewport.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,6 @@ const getViewportModule = () => {
A simplified version of the tracked `OHIFCornerstoneViewport` is shown below, which
creates a cornerstone viewport:

:::note Tip

Not in OHIF version 3.1 we use `displaySets` in the props which is new compared to
the previous version (3.0) which uses `displaySet`. This is due to the fact that
we are moving to a new data model that can render fused images in a single viewport.

:::

```jsx
function TrackedCornerstoneViewport({
Expand Down Expand Up @@ -87,6 +80,45 @@ function TrackedCornerstoneViewport({
}
```

### Viewport re-rendering optimizations

We make use of the React memoization pattern to prevent unnecessary re-renders
for the viewport unless certain aspects of the Viewport props change. You can take
a look into the `areEqual` function in the `OHIFCornerstoneViewport` component to
see how this is done.

```js
function areEqual(prevProps, nextProps) {
if (prevProps.displaySets.length !== nextProps.displaySets.length) {
return false;
}

if (
prevProps.viewportOptions.orientation !==
nextProps.viewportOptions.orientation
) {
return false;
}

// rest of the code
```
as you see, we check if the `needsRerendering` prop is true, and if so, we will
re-render the viewport if the `displaySets` prop changes or the orientation
changes.
We use viewportId to identify a viewport and we use it as a key in React
rendering. This is important because it allows us to keep track of the viewport
and its state, and also let React optimize and move the viewport around in the
grid without re-rendering it. However, there are some cases where we need to
force re-render the viewport, for example, when the viewport is hydrated
with a new Segmentation. For these cases, we use the `needsRerendering` prop
to force re-render the viewport. You can add it to the `viewportOptions`
### `@ohif/app`
Expand Down
1 change: 1 addition & 0 deletions platform/ui/src/components/Viewport/Viewport.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,4 @@ const Viewport: React.FC<ViewportProps> = ({
};

export default Viewport;
export type { ViewportProps, StudyData };
2 changes: 0 additions & 2 deletions platform/ui/src/components/Viewport/index.js

This file was deleted.

5 changes: 5 additions & 0 deletions platform/ui/src/components/Viewport/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import Viewport from './Viewport';
import type { StudyData, ViewportProps } from './Viewport';

export default Viewport;
export type { StudyData, ViewportProps };
2 changes: 1 addition & 1 deletion platform/ui/src/contextProviders/ViewportGridProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export function ViewportGridProvider({ children, service }) {

// If empty viewportOptions, we use numRow and numCols to calculate number of viewports
const hasOptions = layoutOptions?.length;
const viewports = new Map() as Map<string, Viewport>;
const viewports = new Map<string, Viewport>();

// Options is a temporary state store which can be used by the
// findOrCreate to store state about already found viewports. Typically,
Expand Down
10 changes: 5 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -16451,16 +16451,16 @@ prettier-linter-helpers@^1.0.0:
dependencies:
fast-diff "^1.1.2"

prettier@^1.18.2:
version "1.19.1"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.19.1.tgz#f7d7f5ff8a9cd872a7be4ca142095956a60797cb"
integrity sha512-s7PoyDv/II1ObgQunCbB9PdLmUcBZcnWOcxDh7O0N/UwDEsHyqkW+Qh28jW+mVuCdx7gLB0BotYI1Y6uI9iyew==

prettier@^2.8.0:
version "2.8.8"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.8.8.tgz#e8c5d7e98a4305ffe3de2e1fc4aca1a71c28b1da"
integrity sha512-tdN8qQGvNjw4CHbY+XXk0JgCXn9QiF21a55rBe5LJAU+kDyC4WQn4+awm2Xfk2lQMk5fKup9XgzTZtGkjBdP9Q==

prettier@^3.0.3:
version "3.0.3"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-3.0.3.tgz#432a51f7ba422d1469096c0fdc28e235db8f9643"
integrity sha512-L/4pUDMxcNa8R/EthV08Zt42WBO4h1rarVtK0K+QJG0X187OLo7l699jWw0GKuwzkPQ//jMFA/8Xm6Fh3J/DAg==

pretty-bytes@^5.3.0, pretty-bytes@^5.4.1, pretty-bytes@^5.6.0:
version "5.6.0"
resolved "https://registry.yarnpkg.com/pretty-bytes/-/pretty-bytes-5.6.0.tgz#356256f643804773c82f64723fe78c92c62beaeb"
Expand Down

0 comments on commit 50d0d1e

Please sign in to comment.