Skip to content

I2375 added active voxels in volume render#1630

Merged
ABSitf merged 4 commits intomasterfrom
i2375_active_voxels
Sep 22, 2023
Merged

I2375 added active voxels in volume render#1630
ABSitf merged 4 commits intomasterfrom
i2375_active_voxels

Conversation

@ABSitf
Copy link
Contributor

@ABSitf ABSitf commented Sep 21, 2023

No description provided.

Comment on lines +99 to +100
const VoxelBitSet& getVolumeRenderActiveVoxels() const { return volumeRenderActiveVoxels_; }
MRMESH_API void setVolumeRenderActiveVoxels( const VoxelBitSet& activeVoxels );
Copy link
Contributor

Choose a reason for hiding this comment

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

please add comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment on lines +243 to +246
volumeRenderActiveVoxels_.clear();
auto newDims = activeBox_.size();
volumeRenderActiveVoxels_.resize( newDims.x * newDims.y * newDims.z, true );
dirty_ |= DIRTY_SELECTION;
Copy link
Contributor

Choose a reason for hiding this comment

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

why make it here? better in prepareDataForVolumeRendering

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we change active box, it can change box size, so if we try to get active voxels, we will get wrong data

Comment on lines +42 to +43
volumeRenderActiveVoxels_.clear();
volumeRenderActiveVoxels_.resize( vdbVolume_.dims.x * vdbVolume_.dims.y * vdbVolume_.dims.z, true );
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we construct object after creating and try get active voxels, we don't expect empty bitset

Comment on lines +63 to +64
volumeRenderActiveVoxels_.clear();
volumeRenderActiveVoxels_.resize( vdbVolume_.dims.x * vdbVolume_.dims.y * vdbVolume_.dims.z, true );
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same

namespace MR
{
class RenderVolumeObject : public IRenderObject
class MRVIEWER_CLASS RenderVolumeObject : public IRenderObject
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need export this class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why we don't need this (unlike others object classes)?

Comment on lines +23 to +29
protected:
Vector2i activeVoxelsTextureSize_;

MRVIEWER_API RenderBufferRef<unsigned> loadActiveVoxelsTextureBuffer_();

GlTexture2 activeVoxelsTex_;
int maxTexSize_{ 0 };
Copy link
Contributor

Choose a reason for hiding this comment

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

why protected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment on lines +167 to +168
uint dimsXY = uint( dims.y * dims.x );
uint dimsX = uint( dims.x );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
uint dimsXY = uint( dims.y * dims.x );
uint dimsX = uint( dims.x );
uint volumeSize = uint( dims.z *dims.y * dims.x );
uint dimsXY = uint( dims.y*dims.x );

Copy link
Contributor Author

@ABSitf ABSitf Sep 21, 2023

Choose a reason for hiding this comment

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

see next


{
ivec2 texSize = textureSize( activeVoxels, 0 );
uint voxelId = uint( textCoord.z * dims.z ) * dimsXY + uint( textCoord.y * dims.y ) * dimsX + uint( textCoord.x * dims.x );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
uint voxelId = uint( textCoord.z * dims.z ) * dimsXY + uint( textCoord.y * dims.y ) * dimsX + uint( textCoord.x * dims.x );
uint voxelId = uint( textCoord.z * volumeSize ) + uint( textCoord.y * dimsXY ) + uint( textCoord.x * dims.x );

Copy link
Contributor Author

@ABSitf ABSitf Sep 21, 2023

Choose a reason for hiding this comment

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

textCoord is not voxel center (textCoord = voxelCenter + smallShift)
so uint( textCoord.z * volumeSize ) have additional error

Copy link
Contributor

Choose a reason for hiding this comment

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

there can be uint overflow for big volumes voxel index it makes sanse to calculate voxel index directly

@ABSitf ABSitf merged commit 0b4c4e6 into master Sep 22, 2023
@ABSitf ABSitf deleted the i2375_active_voxels branch September 22, 2023 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants