Skip to content
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

fix shader compilation error and invisible color layers in certain circumstances #4556

Merged
merged 4 commits into from
Apr 21, 2020

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Apr 20, 2020

This PR fixes two bugs (both mentioned in #4553).

One issue was a === comparison between a boolean and undefined (I thought, the flow linter would warn about this, but unfortunately it doesn't. we disabled sketchy-bool checks because it also warns about a lot of other stuff, which is usually fine for is).

The other issue was that the segmentation layer name could slip into the colorLayerNames variable. The reason for this was that the LRU layer could contain the segmentation layer. I made the naming clearer and excluded the segmentation layer from the LRU system (since the shader s always compiled for thesegmentation layer; one could re-think this, but that's outside the scope of this PR imo).

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • I created a dataset which contains too many layers to render all at once and a segmentation layer
  • I also ensured that toggling the segmentation layer doesn't throw an error anymore and that the color layer can now be rendered.
    I think the case in question doesn't need to be tested again.

Issues:


@philippotto philippotto self-assigned this Apr 20, 2020
];

return enabledLayerNames
.concat(this.leastRecentlyVisibleLayers)
const names = enabledLayerNames
Copy link
Member Author

Choose a reason for hiding this comment

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

this change makes debugging a bit easier.

@@ -224,7 +224,7 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps> {
<span style={{ fontWeight: 700 }}>
{!isColorLayer && isVolumeTracing ? "Volume Layer" : layerName}
</span>
<Tag style={{ cursor: "default", marginLeft: 8 }}>{elementClass} Layer</Tag>
<Tag style={{ cursor: "default", marginLeft: 8 }}>{elementClass}</Tag>
Copy link
Member Author

Choose a reason for hiding this comment

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

I think the Layer label is not really necessary.

Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

Thanks for fixing! LGTM if the linting errors are fixed 👍

philippotto and others added 2 commits April 21, 2020 11:14
@bulldozer-boy bulldozer-boy bot merged commit ccfd33b into master Apr 21, 2020
@bulldozer-boy bulldozer-boy bot deleted the fix-invisible-layers branch April 21, 2020 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non-segmentation layers are not rendered
2 participants