Skip to content

Commit

Permalink
feat: Use vtk upstream copy of half-voxel fix
Browse files Browse the repository at this point in the history
use new fix from jadh4v

diff image log

update yarn lock

fix compareImage usage in rectangle and sphere scisor tests

fix typos and file naming

Remove existing half-voxel addition changes in vtkStreamingOpenGLVolumeMapper

Add some upstream changes to buildBufferObjects

try switching volumeActorCorners to use center of voxels, not corners

all tests passing with new halfVoxel and outline fixes

fix: worldToIndex should be integer index

Cleanup

remove unnecessary changes
  • Loading branch information
sedghi authored and swederik committed Mar 22, 2022
1 parent f0f96de commit 666596e
Show file tree
Hide file tree
Showing 38 changed files with 665 additions and 626 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ defaults: &defaults
working_directory: ~/repo
# https://circleci.com/docs/2.0/circleci-images/#language-image-variants
docker:
- image: cimg/node:14.16.1-browsers
- image: cimg/node:14.17.0-browsers
environment:
TERM: xterm # Enable colors in term

Expand Down
3 changes: 2 additions & 1 deletion packages/cornerstone-render/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
},
"peerDependencies": {
"gl-matrix": "^3.4.3",
"vtk.js": "git+https://github.com/jadh4v/vtk-js.git#image-volume-half-voxel"
"vtk.js": "git+https://github.com/jadh4v/vtk-js.git#image-volume-half-voxel",
"detect-gpu": "^4.0.7"
},
"dependencies": {
"resemblejs": "^3.2.5"
Expand Down
21 changes: 16 additions & 5 deletions packages/cornerstone-render/src/RenderingEngine/Viewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -493,10 +493,23 @@ class Viewport implements IViewport {

const bounds = renderer.computeVisiblePropBounds()
const focalPoint = [0, 0, 0]
const imageData = this.getDefaultImageData()

// Todo: remove this, this is just for tests passing
if (imageData) {
const spc = imageData.getSpacing()

bounds[0] = bounds[0] + spc[0] / 2
bounds[1] = bounds[1] - spc[0] / 2
bounds[2] = bounds[2] + spc[1] / 2
bounds[3] = bounds[3] - spc[1] / 2
bounds[4] = bounds[4] + spc[2] / 2
bounds[5] = bounds[5] - spc[2] / 2
}

const activeCamera = this.getVtkActiveCamera()
const viewPlaneNormal = activeCamera.getViewPlaneNormal()
const viewUp = activeCamera.getViewUp()
const viewPlaneNormal = <Point3>activeCamera.getViewPlaneNormal()
const viewUp = <Point3>activeCamera.getViewUp()

// Reset the perspective zoom factors, otherwise subsequent zooms will cause
// the view angle to become very small and cause bad depth sorting.
Expand All @@ -507,8 +520,6 @@ class Viewport implements IViewport {
focalPoint[1] = (bounds[2] + bounds[3]) / 2.0
focalPoint[2] = (bounds[4] + bounds[5]) / 2.0

const imageData = this.getDefaultImageData()

if (imageData) {
const dimensions = imageData.getDimensions()
const middleIJK = dimensions.map((d) => Math.floor(d / 2))
Expand Down Expand Up @@ -694,7 +705,7 @@ class Viewport implements IViewport {
return {
viewUp: <Point3>vtkCamera.getViewUp(),
viewPlaneNormal: <Point3>vtkCamera.getViewPlaneNormal(),
clippingRange: <Point3>vtkCamera.getClippingRange(),
clippingRange: <Point2>vtkCamera.getClippingRange(),
// TODO: I'm really not sure about this, it requires a calculation, and
// how useful is this without the renderer context?
// Lets add it back if we find we need it.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import macro from 'vtk.js/Sources/macro'
import macro from 'vtk.js/Sources/macros'
import vtkStreamingOpenGLRenderWindow from './vtkStreamingOpenGLRenderWindow'
import vtkRenderer from 'vtk.js/Sources/Rendering/Core/Renderer'
import vtkRenderWindow from 'vtk.js/Sources/Rendering/Core/RenderWindow'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import macro from 'vtk.js/Sources/macro'
import macro from 'vtk.js/Sources/macros'
import vtkVolumeMapper from 'vtk.js/Sources/Rendering/Core/VolumeMapper'

/**
* vtkSharedVolumeMapper - A dervied class of the core vtkVolumeMapper class
* vtkSharedVolumeMapper - A derived class of the core vtkVolumeMapper class
* the scalar texture in as an argument. This is so we can share the same texture
* memory across different mappers/actors, so we don't duplicate memory usage.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { mat4 } from 'gl-matrix'
import { VtkObject } from 'vtk.js/Sources/macro'
import { VtkObject } from 'vtk.js/Sources/macros'

// Copied from VTKCamera

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import macro from 'vtk.js/Sources/macro'
import macro from 'vtk.js/Sources/macros'
import vtkCamera from 'vtk.js/Sources/Rendering/Core/Camera'
import { vec3, mat4 } from 'gl-matrix'
import vtkMath from 'vtk.js/Sources/Common/Core/Math'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import macro from 'vtk.js/Sources/macro'
import macro from 'vtk.js/Sources/macros'
import vtkOpenGLRenderWindow from 'vtk.js/Sources/Rendering/OpenGL/RenderWindow'
import vtkStreamingOpenGLViewNodeFactory from './vtkStreamingOpenGLViewNodeFactory'

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import macro from 'vtk.js/Sources/macro'
import macro from 'vtk.js/Sources/macros'
import vtkOpenGLTexture from 'vtk.js/Sources/Rendering/OpenGL/Texture'

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import macro from 'vtk.js/Sources/macro'
import macro from 'vtk.js/Sources/macros'
// import vtkGenericWidgetRepresentation from 'vtk.js/Sources/Rendering/SceneGraph/GenericWidgetRepresentation'
import vtkOpenGLActor from 'vtk.js/Sources/Rendering/OpenGL/Actor'
import vtkOpenGLActor2D from 'vtk.js/Sources/Rendering/OpenGL/Actor2D'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { mat3, mat4, vec3 } from 'gl-matrix'
import macro from 'vtk.js/Sources/macro'
import macro from 'vtk.js/Sources/macros'
import vtkOpenGLVolumeMapper from 'vtk.js/Sources/Rendering/OpenGL/VolumeMapper'
import { Filter } from 'vtk.js/Sources/Rendering/OpenGL/Texture/Constants'
import { VtkDataTypes } from 'vtk.js/Sources/Common/Core/DataArray/Constants'
Expand All @@ -11,7 +11,7 @@ const { vtkWarningMacro } = macro
/**
* vtkStreamingOpenGLVolumeMapper - A dervied class of the core vtkOpenGLVolumeMapper class.
* This class replaces the buildBufferObjects function so that we progressively upload our textures
* into GPU memory uisng the new methods on vtkStreamingOpenGLTexture.
* into GPU memory using the new methods on vtkStreamingOpenGLTexture.
*
*
* @param {*} publicAPI The public API to extend
Expand All @@ -32,8 +32,12 @@ function vtkStreamingOpenGLVolumeMapper(publicAPI, model) {
*/
publicAPI.buildBufferObjects = (ren, actor) => {
const image = model.currentInput
if (!image) {
return
}

if (image === null) {
const scalars = image.getPointData() && image.getPointData().getScalars()
if (!scalars) {
return
}

Expand All @@ -55,7 +59,7 @@ function vtkStreamingOpenGLVolumeMapper(publicAPI, model) {
)
}

const numComp = image.getPointData().getScalars().getNumberOfComponents()
const numComp = scalars.getNumberOfComponents()
const iComps = vprop.getIndependentComponents()
const numIComps = iComps ? numComp : 1

Expand Down Expand Up @@ -190,8 +194,9 @@ function vtkStreamingOpenGLVolumeMapper(publicAPI, model) {
dims[1],
dims[2],
numComp,
dataType,
data
scalars.getDataType(),
scalars.getData(),
model.renderable.getPreferSizeOverAccuracy()
)
} else {
model.scalarTexture.deactivate()
Expand Down Expand Up @@ -274,16 +279,6 @@ function vtkStreamingOpenGLVolumeMapper(publicAPI, model) {
const spc = model.currentInput.getSpacing()
const dims = model.currentInput.getDimensions()

// TODO: need a better name because this is not physical?
// TODO: this should probably use extent, not bounds?
const physicalBounds = [...bounds]
physicalBounds[0] -= 0.5 * spc[0]
physicalBounds[1] += 0.5 * spc[0]
physicalBounds[2] -= 0.5 * spc[1]
physicalBounds[3] += 0.5 * spc[1]
physicalBounds[4] -= 0.5 * spc[2]
physicalBounds[5] += 0.5 * spc[2]

// compute the viewport bounds of the volume
// we will only render those fragments.
const pos = new Float64Array(3)
Expand All @@ -296,9 +291,9 @@ function vtkStreamingOpenGLVolumeMapper(publicAPI, model) {
for (let i = 0; i < 8; ++i) {
vec3.set(
pos,
physicalBounds[i % 2],
physicalBounds[2 + (Math.floor(i / 2) % 2)],
physicalBounds[4 + Math.floor(i / 4)]
bounds[i % 2],
bounds[2 + (Math.floor(i / 2) % 2)],
bounds[4 + Math.floor(i / 4)]
)
vec3.transformMat4(pos, pos, model.modelToView)
if (!cam.getParallelProjection()) {
Expand Down Expand Up @@ -330,13 +325,13 @@ function vtkStreamingOpenGLVolumeMapper(publicAPI, model) {
program.setUniformi('cameraParallel', cam.getParallelProjection())
}

const ext = model.currentInput.getExtent()
const ext = model.currentInput.getSpatialExtent()
const vsize = new Float64Array(3)
vec3.set(
vsize,
(ext[1] - ext[0] + 1) * spc[0],
(ext[3] - ext[2] + 1) * spc[1],
(ext[5] - ext[4] + 1) * spc[2]
(ext[1] - ext[0]) * spc[0],
(ext[3] - ext[2]) * spc[1],
(ext[5] - ext[4]) * spc[2]
)
program.setUniform3f('vSpacing', spc[0], spc[1], spc[2])

Expand All @@ -348,12 +343,6 @@ function vtkStreamingOpenGLVolumeMapper(publicAPI, model) {

program.setUniform3f('vOriginVC', pos[0], pos[1], pos[2])

vec3.set(pos, ext[0] + 0.5, ext[2] + 0.5, ext[4] + 0.5)
model.currentInput.indexToWorldVec3(pos, pos)

vec3.transformMat4(pos, pos, model.modelToView)
program.setUniform3f('vOriginPlusHalfVoxelVC', pos[0], pos[1], pos[2])

// apply the image directions
const i2wmat4 = model.currentInput.getIndexToWorld()
mat4.multiply(model.idxToView, model.modelToView, i2wmat4)
Expand Down Expand Up @@ -401,30 +390,30 @@ function vtkStreamingOpenGLVolumeMapper(publicAPI, model) {
const pos2 = new Float64Array(3)
for (let i = 0; i < 6; ++i) {
switch (i) {
default:
case 0:
vec3.set(normal, 1.0, 0.0, 0.0)
vec3.set(pos2, ext[1] + 0.5, ext[3] + 0.5, ext[5] + 0.5)
break
case 1:
vec3.set(normal, -1.0, 0.0, 0.0)
vec3.set(pos2, ext[0] - 0.5, ext[2] - 0.5, ext[4] - 0.5)
vec3.set(pos2, ext[0], ext[2], ext[4])
break
case 2:
vec3.set(normal, 0.0, 1.0, 0.0)
vec3.set(pos2, ext[1] + 0.5, ext[3] + 0.5, ext[5] + 0.5)
vec3.set(pos2, ext[1], ext[3], ext[5])
break
case 3:
vec3.set(normal, 0.0, -1.0, 0.0)
vec3.set(pos2, ext[0] - 0.5, ext[2] - 0.5, ext[4] - 0.5)
vec3.set(pos2, ext[0], ext[2], ext[4])
break
case 4:
vec3.set(normal, 0.0, 0.0, 1.0)
vec3.set(pos2, ext[1] + 0.5, ext[3] + 0.5, ext[5] + 0.5)
vec3.set(pos2, ext[1], ext[3], ext[5])
break
case 5:
vec3.set(normal, 0.0, 0.0, -1.0)
vec3.set(pos2, ext[0] - 0.5, ext[2] - 0.5, ext[4] - 0.5)
vec3.set(pos2, ext[0], ext[2], ext[4])
break
case 0:
default:
vec3.set(normal, 1.0, 0.0, 0.0)
vec3.set(pos2, ext[1], ext[3], ext[5])
break
}
vec3.transformMat3(normal, normal, model.idxNormalMatrix)
Expand Down Expand Up @@ -508,17 +497,33 @@ function vtkStreamingOpenGLVolumeMapper(publicAPI, model) {
}
}

// publicAPI.getRenderTargetSize = () => {
// // https://github.com/Kitware/vtk-js/blob/master/Sources/Rendering/OpenGL/VolumeMapper/index.js#L952
// if (model.lastXYF > 1.43) {
// const sz = model.framebuffer.getSize()
// return [model.fvp[0] * sz[0], model.fvp[1] * sz[1]]
// }

// // This seems wrong, it assumes the renderWindow only has one renderer
// // but I don't know if this stuff is correct...

// const { usize, vsize } = model.openGLRenderer.getTiledSizeAndOrigin()

// return [usize, vsize]
// }

// publicAPI.getRenderTargetSize = () => {
// if (model._useSmallViewport) {
// return [model._smallViewportWidth, model._smallViewportHeight]
// }

// return model.openGLRenderWindow.getFramebufferSize()
// }
publicAPI.getRenderTargetSize = () => {
// https://github.com/Kitware/vtk-js/blob/master/Sources/Rendering/OpenGL/VolumeMapper/index.js#L952
if (model.lastXYF > 1.43) {
const sz = model.framebuffer.getSize()
return [model.fvp[0] * sz[0], model.fvp[1] * sz[1]]
if (model._useSmallViewport) {
return [model._smallViewportWidth, model._smallViewportHeight]
}

// This seems wrong, it assumes the renderWindow only has one renderer
// but I don't know if this stuff is correct...
// return model.openGLRenderWindow.getFramebufferSize();

const { usize, vsize } = model.openGLRenderer.getTiledSizeAndOrigin()

return [usize, vsize]
Expand Down
3 changes: 2 additions & 1 deletion packages/cornerstone-render/src/types/ICamera.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Point2 from './Point2'
import Point3 from './Point3'

/**
Expand All @@ -6,7 +7,7 @@ import Point3 from './Point3'
*/
interface ICamera {
/** Camera Clipping range*/
clippingRange?: Point3
clippingRange?: Point2
/** Camera Focal point */
focalPoint?: Point3
/** Camera Parallel Projection flag - whether camera is using parallel projection */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import { Point3 } from '../types'
* @returns An array of the corners of the `volumeActor` in world space.
*/
export default function getVolumeActorCorners(volumeActor): Array<Point3> {
const bounds = volumeActor.getMapper().getBounds()
const imageData = volumeActor.getMapper().getInputData()
const bounds = imageData.extentToBounds(imageData.getExtent())

return [
[bounds[0], bounds[2], bounds[4]],
Expand Down
7 changes: 4 additions & 3 deletions packages/cornerstone-render/src/utilities/testUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,11 @@ function compareImages(imageDataURL, baseline, outputName) {
// and download the difference image
// Todo: this should be a configurable threshold
if (mismatch > 1) {
console.log(imageDataURL)

console.log('mismatch of ' + mismatch + '%')
const diff = data.getImageDataUrl()
console.log('mismatch of ' + mismatch + '%')
console.log('diff image is')
console.log(diff)

// Todo: we should store the diff image somewhere

reject(new Error(`mismatch between images for ${outputName}`))
Expand Down
10 changes: 5 additions & 5 deletions packages/cornerstone-tools/src/tools/CrosshairsTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ export default class CrosshairsTool extends AnnotationTool {
} => {
const enabledElement = getEnabledElementByUIDs(
viewportUID,
renderingEngineUID,
renderingEngineUID
)
const { FrameOfReferenceUID, viewport } = enabledElement
const { element } = viewport
Expand Down Expand Up @@ -748,12 +748,12 @@ export default class CrosshairsTool extends AnnotationTool {
direction
)
vtkMath.normalize(direction)
vtkMath.multiplyScalar(<vec3>direction, otherCanvasDiagonalLength)
vtkMath.multiplyScalar(<Types.Point3>direction, otherCanvasDiagonalLength)

const pointWorld0: Types.Point3 = [0, 0, 0]
vtkMath.add(otherViewportCenterWorld, direction, pointWorld0)

const pointWorld1 = [0, 0, 0]
const pointWorld1: Types.Point3 = [0, 0, 0]
vtkMath.subtract(otherViewportCenterWorld, direction, pointWorld1)

// get canvas information for points and lines (canvas box, canvas horizontal distances)
Expand Down Expand Up @@ -881,7 +881,7 @@ export default class CrosshairsTool extends AnnotationTool {
}

// SlabThickness center in world
let stHandlesCenterWorld = [...this.toolCenter]
let stHandlesCenterWorld: Types.Point3 = [...this.toolCenter]
if (
!otherViewportDraggableRotatable &&
otherViewportSlabThicknessControlsOn
Expand Down Expand Up @@ -2083,7 +2083,7 @@ export default class CrosshairsTool extends AnnotationTool {
otherViewportRotationPoints[1][3]
)
vtkMath.add(point1, point2, currentCenter)
vtkMath.multiplyScalar(<vec3>currentCenter, 0.5)
vtkMath.multiplyScalar(<Types.Point3>currentCenter, 0.5)
}
}

Expand Down
Loading

0 comments on commit 666596e

Please sign in to comment.