Skip to content
This repository has been archived by the owner on Jul 8, 2022. It is now read-only.

Fix 16-bits and anisotropic raw volumes bugs #358

Merged
merged 1 commit into from
Dec 22, 2016

Conversation

chevtche
Copy link
Contributor

No description provided.

Copy link
Contributor

@tribal-tec tribal-tec left a comment

Choose a reason for hiding this comment

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

+1, except for missing Changelog update

@@ -81,15 +75,13 @@ class NoMemoryUnit : public MemoryUnit
class ConstMemoryUnit : public MemoryUnit
{
public:
ConstMemoryUnit( const uint8_t* ptr, const size_t size );
explicit ConstMemoryUnit( const uint8_t* ptr );
Copy link

Choose a reason for hiding this comment

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

How is this possible that the size is unused? With a raw pointer you always need a size, the only exception I know of is the '\0' terminated c_str, but no such convention seems to be used / documented here...

Copy link

Choose a reason for hiding this comment

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

So as per our discussion, the whole MemoryUnit classes should be rewritten, but this is a bigger story. Meanwhile, this "cleanup" is OK as it highlights the underlying design issues.

@@ -253,6 +233,50 @@ struct RayCastRenderer::Impl
_computedSamplesPerRay = std::max( maxVoxelsAtLOD, (float)minSamplesPerRay );
}

Vector2f dataSourceRange;
uint32_t shaderDataType;
switch( _dataSource.getVolumeInfo().dataType )
Copy link

Choose a reason for hiding this comment

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

this switch is too long to be inlined and should be moved back to one or two separate methods.

@chevtche chevtche merged commit cb572a5 into BlueBrain:master Dec 22, 2016
@eile eile mentioned this pull request Dec 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants