bumpy flatmap fixes - #310
Closed
alexhuth wants to merge 2 commits into
Closed
Conversation
Contributor
Author
|
btw papers exist on the topic of topographic shading and they are intense https://www.researchgate.net/profile/Bernhard_Jenny/publication/233684103_Swiss-Style_Colour_Relief_Shading_Modulated_by_Elevationand_by_Exposure_to_Illumination/links/568082d508ae051f9ae7cdd6/Swiss-Style-Colour-Relief-Shading-Modulated-by-Elevationand-by-Exposure-to-Illumination.pdf |
Contributor
|
screenshot of the new changes looks pretty nice. could you post one with actual data on it? e.g. rgb semantic pcs flatmaps are very very difficult to build an intuition for. I strongly agree that we need to continue innovating in that area |
Contributor
|
I suspect much of that justification was aimed at me, since I have vocally objected to specularity and bumps. Fair points, I will stop complaining, this is reasonably useful (but I will still leave them off by default). I would also prefer to address the dimness issue before merging (does it only affect the lighting with the bumps on? or all the time? if all the time, then I strongly prefer fixing before merge.) Also - any chance that while you're procrastinating you could address #286 ?? (also a lighting issue in there) |
This was referenced Aug 20, 2026
Contributor
alexhuth
added a commit
that referenced
this pull request
Aug 21, 2026
`smoothfactor` and `smoothiter` were declared with `var` inside the `if (this.flatlims !== undefined)` block but used earlier in the same per-hemisphere loop, to smooth `wmareas` and `pialareas`. Hoisting made the names visible there but left them `undefined` on the first iteration, so `iterativelySmoothVertexData` ran `for (i = 0; i < undefined; i++)` -- zero iterations -- and returned the areas unsmoothed. The second iteration then saw the values assigned during the first, so the left hemisphere was unsmoothed and the right was smoothed, giving the two hemispheres visibly different bump relief. Declare the parameters above the loop, split into separate constants for the area and distance passes, and reduce the iteration counts (areas 50 -> 5, dists 50 -> 20) for a somewhat less smoothed result. Split out of #310. Co-authored-by: alexhuth <alex.huth@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
alexhuth
added a commit
that referenced
this pull request
Aug 21, 2026
`smoothfactor` and `smoothiter` were declared with `var` inside the `if (this.flatlims !== undefined)` block but used earlier in the same per-hemisphere loop, to smooth `wmareas` and `pialareas`. Hoisting made the names visible there but left them `undefined` on the first iteration, so `iterativelySmoothVertexData` ran `for (i = 0; i < undefined; i++)` -- zero iterations -- and returned the areas unsmoothed. The second iteration then saw the values assigned during the first, so the left hemisphere was unsmoothed and the right was smoothed, giving the two hemispheres visibly different bump relief. Declare the parameters above the loop, split into separate constants for the area and distance passes, and reduce the iteration counts (areas 50 -> 5, dists 50 -> 20) for a somewhat less smoothed result. Split out of #310. Co-authored-by: alexhuth <alex.huth@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
mvdoc
added a commit
that referenced
this pull request
Aug 21, 2026
Adds an opt-in single grazing light from the upper left, following the top-left lighting convention used for shaded-relief topographic maps. This makes sulci read as valleys and gyri as ridges, which is what makes bumpy flatmaps legible. #310 made this the unconditional default, but the lighting setup applies to every view, not just flatmaps, while bumpy_flatmap itself defaults to false -- and a single near-tangential light is noticeably dimmer than the existing three-light setup. Putting it behind a config flag alongside uniform_illumination keeps the default rendering untouched. Default and missing-key paths produce the same three lights, positions and intensities as before. Split out of #310. Co-Authored-By: alexhuth <alex.huth@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mvdoc
added a commit
that referenced
this pull request
Aug 21, 2026
* NF: add webgl_viewopts.topleft_lighting config option Adds an opt-in single grazing light from the upper left, following the top-left lighting convention used for shaded-relief topographic maps. This makes sulci read as valleys and gyri as ridges, which is what makes bumpy flatmaps legible. #310 made this the unconditional default, but the lighting setup applies to every view, not just flatmaps, while bumpy_flatmap itself defaults to false -- and a single near-tangential light is noticeably dimmer than the existing three-light setup. Putting it behind a config flag alongside uniform_illumination keeps the default rendering untouched. Default and missing-key paths produce the same three lights, positions and intensities as before. Split out of #310. Co-Authored-By: alexhuth <alex.huth@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * corrected top-left lighting * rationalized lighting * FIX: bumpy_flatmap had no effect on vertex data The bump displacement and the flatBumpNorms normal perturbation only ever existed in Shaders.surface_pixel, so toggling bumpy_flatmap while showing a Vertex dataview did nothing at all: no HASFLAT define, no flatheight attribute, and vNormal left as the unperturbed surface normal, which is what made the flatmap read as a flat sheet even though the geometry underneath it had already been displaced. Port the HASFLAT blocks from surface_pixel into surface_vertex verbatim, so the two shaders now displace and shade the flatmap identically and the new lighting controls behave the same way for volume and vertex data. Note that the pick and depth shaders still use the old thickness-based displacement and are unaware of the bump, so picking on a bumpy flatmap tests against undisplaced geometry. That was already true for volume data before this change and is left alone here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: alexhuth <alex.huth@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Alexander Huth <huth@phlegmatic3000-102.local>
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
4 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.
To (~) productively procrastinate from writing, I fixed some issues with the bumpy flatmap rendering. First let me remind y'all what the point of bumpy flatmaps is.
FLATMAPS ARE HARD TO READ. People (current audience excluded) often have no fuuucking clue what they're looking at on a flatmap, because flatmaps are typically missing landmarks. This is especially bad for flatmaps with un-thresholded data, because then the typical binary curvature map (which can help orient folks) is missing as well. We try to alleviate these problems by adding ROI outlines and lines indicating sulci & gyri. But I don't think that's sufficient.
The goal of bumpy flatmaps is to give subtle visual cues (through shading and, if you're into that kind of thing, specularity) that show where sulci and gyri are. I think it works kinda well?
The changes in this branch:
I fixed a bug where the bump height was being smoothed in the right hemisphere but not the left. I also reduced the amount of smoothing somewhat. The amount of smoothing can be tweaked if it's currently too rough for peoples' preferences.
I tweaked the lighting (which affects not only the flatmap but other views as well) to follow the top-left lighting convention for topographic maps, which imo makes it much more obvious that sulci are "valleys" and gyri are "mountains". This also had the effect of making the lighting dimmer overall, which is not ideal but we can fix.
An example:
Before these changes
After these changes
