Refactor cortical sheet displacement into reusable shader utility - #708
Open
alexhuth wants to merge 1 commit into
Open
Refactor cortical sheet displacement into reusable shader utility#708alexhuth wants to merge 1 commit into
alexhuth wants to merge 1 commit into
Conversation
The pick and depth shaders hardcoded the thickness-based cortical-sheet displacement (.62 * thickness) and knew nothing about HASFLAT or the bumpyflat uniform, so on any subject with a flatmap they tested against geometry that isn't the geometry being drawn: bumpy_flatmap on -> surface bumps by flatheight, pick/depth used .62*thickness bumpy_flatmap off -> surface isn't displaced at all, pick/depth still displaced The second row is the worse one, and it isn't about bumpy flatmaps: on a partially unfolded surface with the bump off, the picker pushed vertices 0.62 * thickness along their normals while the surface stayed put, which shows up as picking that lands a few pixels off (worst near silhouette edges, where the normal is perpendicular to the view direction) and as ROI labels occluded against slightly the wrong surface. Rather than add a fourth copy of the displacement expression -- four copies is how the shaders drifted apart to begin with -- pull it into utils.cortsheet_displace and have all four shaders emit it, along with utils.bumpyflat for the uniform and attribute it needs. pick and depth now take a hasflat opt, bind the flatheight attribute, and share the surface's bumpyflat uniform object so they follow the toggle without listening for it. The GLSL the surface shaders emit is unchanged apart from where the flatheight declaration sits. Fixes #704 Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Tr6MczRdE3ad5ss4FcsRcV
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR refactors duplicated cortical sheet displacement logic in vertex shaders into a reusable utility function, and ensures the bumpy flatmap feature is consistently applied across all shader variants (pick, depth, and surface shaders).
Key Changes
New shader utilities: Added
bumpyflatheader andcortsheet_displace()function toShaderlibto eliminate code duplicationbumpyflat: Declares uniform and attribute variables needed for bumpy flatmap displacementcortsheet_displace(morphs, thickmix): Encapsulates the identical displacement calculation that was previously copied across multiple shadersUnified shader construction: Replaced inline displacement code in three shader variants with calls to
utils.cortsheet_displace():Consistent hasflat support: Added
hasflatoption to shader builders to ensure pick and depth shaders match the surface shader's flatmap configurationhasflat:surf.flatlims !== undefinedhasflat:surf.flatlims !== undefinedHASFLATpreprocessor flag when neededShared uniform references: Updated pick and depth shaders to reference
surf.uniforms.bumpyflatdirectly (shared object, not a copy) so they automatically follow bumpy flatmap toggles without explicit event listenersImplementation Details
cortsheet_displace()function is parameterized by morph count and thickmix variable name to support different shader contexts#ifdef CORTSHEETand#ifdef HASFLATis preserved to maintain feature parityflatheightattribute is now only declared whenHASFLATis defined, reducing unnecessary attribute overheadhttps://claude.ai/code/session_01Tr6MczRdE3ad5ss4FcsRcV