Md 476 viv histogram widgets #169
Draft
Conversation
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
try/catch in unproject rather than using onAfterRender that caused infinite loop.
* use container div for deckgl, fix axis update and mouse coordinates * move useScatterplotLayer so that we are always using the same instance * some doc comments on spatial layers
* useScatterRadius hook * somewhat reasonable default pending better relation to units etc
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes update multiple React components and utilities. A new histogram component and range filtering hooks have been added, with state management improvements using hooks and debouncing. The channel components now include raster data in their state and statistics calls, along with a new error display function. Other modifications include revised control flow with explicit block statements, updated hover handling in scatter components, and enhanced type definitions for raster data. Additionally, formatting, comments for future reviews, and minor refactorings have been introduced across several components. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CH as ChannelHistogram
participant WW as WebWorker
participant EB as ErrorBoundary
U->>CH: Adjust histogram control
CH->>CH: Debounce value update
CH->>WW: Request histogram data
WW-->>CH: Return histogram data
CH->>CH: Update contrast limits and state
EB-->>CH: Handle errors if any
sequenceDiagram
participant U as User
participant HC as HistogramComponent
participant D3 as D3 Brush
participant WW as WebWorker
U->>HC: Interact with histogram (brush gesture)
HC->>D3: Initialize brush events
D3-->>HC: Emit range selection event
HC->>HC: Debounce and update range filter state
HC->>WW: Query filtered data
WW-->>HC: Return filtered results
HC->>HC: Render updated histogram
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Collaborator
Author
|
@coderabbitai review, and suggest saner state management in |
Contributor
✅ Actions performedReview triggered.
|
Contributor
🧩 Analysis chain🏁 Script executed: Length of output: 220 🏁 Script executed: Length of output: 20541 🏁 Script executed: Length of output: 277 🏁 Script executed: Length of output: 17292 @xinaesthete I'll help you review and fix the state management in the 🏁 Script executed: Length of output: 24447 @xinaesthete I've identified the cause of the infinite loop in your state management between
Here's a fix for the const ChannelHistogram = ({ index }: { index: number }) => {
const limits = useChannelsStore(({ contrastLimits }) => contrastLimits);
const { domains, raster } = useChannelsStore(({ domains, raster }) => ({ domains, raster }));
const domain = domains[index];
const rasterData = raster[index]?.data || [0];
const [histogramData, setHistogramData] = useState<number[]>([]);
// Add a ref to track if changes are coming from user interaction
const isUserInteraction = useRef(false);
const [min, max] = domain;
const scaleValue = useCallback((v: number) => (v - min) / (max - min), [min, max]);
const { limit, normalisedLow, normalisedHigh } = useMemo(() => {
const limit = limits[index];
const normalisedLow = scaleValue(limit[0]);
const normalisedHigh = scaleValue(limit[1]);
return { limit, normalisedLow, normalisedHigh };
}, [limits, index, scaleValue]);
const channelsStore = useChannelsStoreApi();
// Histogram data query logic
const queryHistogram = useCallback(async () => {
const worker = new Worker(
new URL("../../datastore/rawHistogramWorker.ts", import.meta.url),
);
worker.onmessage = (event) => {
setHistogramData(event.data);
worker.terminate();
};
const isInt32 = false;
const originalData = rasterData;
const data = new SharedArrayBuffer(originalData.length * 4);
new Float32Array(data).set(originalData);
worker.postMessage({
data,
min,
max,
bins: 100,
isFloat: isInt32,
});
}, [min, max, rasterData]);
// Use a more substantial debounce time
const [liveValue, setLiveValue] = useState<Range | null>(null);
const debounceTime = 50; // Increased from 10
const [debouncedValue] = useDebounce(liveValue, debounceTime);
useEffect(() => {
// Only update store if this came from user interaction
if (!isUserInteraction.current) return;
if (!debouncedValue) return;
if (debouncedValue.some(Number.isNaN)) return;
// Deep equality check
const currentLimits = channelsStore.getState().contrastLimits;
const currentLimit = currentLimits[index];
if (
currentLimit &&
Math.abs(debouncedValue[0] - currentLimit[0]) < 0.0001 &&
Math.abs(debouncedValue[1] - currentLimit[1]) < 0.0001
) {
return;
}
// Create a new array instead of mutating
const newLimits = [...currentLimits];
newLimits[index] = [...debouncedValue];
// After updating store, reset the flag
isUserInteraction.current = false;
channelsStore.setState({ contrastLimits: newLimits });
}, [debouncedValue, index, channelsStore]);
const isChannelLoading = useViewerStore((state) => state.isChannelLoading)[index];
return (
<div className="p-4">
{!isChannelLoading ? <Histogram
value={limit}
step={0.001}
histogram={histogramData}
lowFraction={normalisedLow}
highFraction={normalisedHigh}
queryHistogram={queryHistogram}
setValue={v => {
// Set the user interaction flag when value comes from the brush
isUserInteraction.current = true;
if (v) setLiveValue(v);
}}
minMax={domain}
histoWidth={100}
histoHeight={50}
/> : "Loading..."}
</div>
);
};And also modify the const useBrushX = (
ref: React.RefObject<SVGSVGElement>,
{ value, setValue, minMax, histoWidth, histoHeight }: RangeProps,
) => {
const brushRef = useRef<ReturnType<typeof d3.brushX> | null>(null);
// Track whether we're currently handling a programmatic update
const isUpdatingProgrammatically = useRef(false);
const [initialValue] = useState(value);
useEffect(() => {
if (!ref.current) return;
const svg = d3.select(ref.current);
// Set up brush
const brush = d3
.brushX()
.handleSize(1)
.extent([
[0, -2],
[histoWidth, histoHeight + 2],
])
.on("brush end", (event) => {
// Skip if this event was triggered programmatically
if (isUpdatingProgrammatically.current) return;
if (event.selection) {
const [start, end] = event.selection.map((x: number) => {
if (!ref.current) {
console.error("No ref.current in brush event handler");
return 0;
}
const { width } = ref.current.getBoundingClientRect();
const r = width / histoWidth;
const normalizedX = (r * x) / width;
return minMax[0] + normalizedX * (minMax[1] - minMax[0]);
});
setValue([start, end]);
} else {
setValue(null);
}
});
brushRef.current = brush;
// Apply the brush to the SVG
const brushGroup = svg.append("g").attr("class", "brush").call(brush);
// Initialize brush selection based on the initial value
if (initialValue) {
isUpdatingProgrammatically.current = true;
const [start, end] = initialValue.map(
(v) => ((v - minMax[0]) / (minMax[1] - minMax[0])) * histoWidth,
);
brushGroup.call(brush.move, [start, end]);
isUpdatingProgrammatically.current = false;
}
// Apply vector effect styling
brushGroup
.selectAll(".selection")
.attr("vector-effect", "non-scaling-stroke");
brushGroup
.selectAll(".handle")
.attr("vector-effect", "non-scaling-stroke");
// Cleanup on unmount
return () => {
svg.select(".brush").remove();
};
}, [ref, setValue, minMax, histoWidth, histoHeight, initialValue]);
// Use a longer debounce time for prop changes
const [debouncedValue] = useDebounce(value, 150, {
equalityFn: (a, b) => {
if (!a && !b) return true;
if (!a || !b) return false;
return a[0] === b[0] && a[1] === b[1];
},
});
const setBrushValue = useCallback<set2d>(
(v) => {
if (!brushRef.current || !ref.current) return;
const svg = d3.select(ref.current);
// Set flag before updating
isUpdatingProgrammatically.current = true;
if (!v) {
svg.select(".brush").call(brushRef.current.move, null);
isUpdatingProgrammatically.current = false;
return;
}
const [start, end] = v;
const x0 = ((start - minMax[0]) / (minMax[1] - minMax[0])) * histoWidth;
const x1 = ((end - minMax[0]) / (minMax[1] - minMax[0])) * histoWidth;
svg.select(".brush").call(brushRef.current.move, [x0, x1]);
// Reset flag after updating
isUpdatingProgrammatically.current = false;
},
[minMax, histoWidth, ref],
);
useEffect(() => {
if (debouncedValue) {
setBrushValue(debouncedValue);
}
}, [debouncedValue, setBrushValue]);
};The key changes to fix the infinite loop:
These changes break the circular dependency by preventing the update cycles, while maintaining the functionality where brush changes update the store and store changes update the brush UI. ✅ Actions performedReview triggered.
|
xinaesthete
force-pushed
the
MD-476-viv-histogram-widgets
branch
from
b5f89f4 to
4eda2f2
Compare
Collaborator
Author
|
@coderabbitai I think we should use tanstack query for |
Contributor
|
Tip For best results, initiate chat on the files or code changes. @xinaesthete Using TanStack Query for your histogram data fetching is an excellent idea! Here's a sketch of how you could implement it: // 1. First, create a worker utility that returns promises
// src/utils/workerUtils.ts
export function createHistogramWorker(data: Float32Array, min: number, max: number, bins: number) {
return new Promise
<number[]>((resolve, reject) => {
try {
const worker = new Worker(
new URL("../../datastore/rawHistogramWorker.ts", import.meta.url)
);
worker.onmessage = (event) => {
resolve(event.data);
worker.terminate();
};
worker.onerror = (error) => {
reject(error);
worker.terminate();
};
const sharedBuffer = new SharedArrayBuffer(data.length * 4);
new Float32Array(sharedBuffer).set(data);
worker.postMessage({
data: sharedBuffer,
min,
max,
bins,
isFloat: true,
});
} catch (err) {
reject(err);
}
});
}
// 2. Create a React Query hook
// src/hooks/useHistogramQuery.ts
import { useQuery } from '@tanstack/react-query';
import { createHistogramWorker } from '../utils/workerUtils';
export function useHistogramQuery(
channelIndex: number,
rasterData: Float32Array,
domain: [number, number],
bins = 100,
enabled = true
) {
return useQuery({
queryKey: ['histogram', channelIndex, domain[0], domain[1], bins, rasterData.length],
queryFn: () => createHistogramWorker(rasterData, domain[0], domain[1], bins),
enabled: enabled && rasterData.length > 0,
staleTime: Infinity, // Data won't get stale unless explicitly invalidated
// Only refetch when rasterData or domain actually changes content
structuralSharing: false,
});
}
// 3. Update the ChannelHistogram component
// ChannelHistogram.tsx
import { useHistogramQuery } from '../hooks/useHistogramQuery';
import { Histogram } from './Histogram'; // Your existing or visx-based histogram component
import { useChannelsStore, useChannelsStoreApi } from '../store/channelsStore';
const ChannelHistogram = ({ index }: { index: number }) => {
const { contrastLimits, domains, raster } = useChannelsStore(state => ({
contrastLimits: state.contrastLimits,
domains: state.domains,
raster: state.raster
}));
const channelsStore = useChannelsStoreApi();
const domain = domains[index] || [0, 1];
const limit = contrastLimits[index] || domain;
const rasterData = raster[index]?.data || new Float32Array();
// Use TanStack Query for histogram data
const {
data: histogramData = [],
isLoading,
isError,
error
} = useHistogramQuery(index, rasterData, domain, 100, true);
const handleRangeChange = useCallback((newLimit: [number, number]) => {
const newLimits = [...contrastLimits];
newLimits[index] = newLimit;
channelsStore.setState({ contrastLimits: newLimits });
}, [channelsStore, contrastLimits, index]);
if (isLoading) return <div className="p-4">Loading histogram...</div>;
if (isError) return <div className="p-4">Error: {error?.message}</div>;
return (
<div className="p-4">
<Histogram
value={limit}
setValue={handleRangeChange}
minMax={domain}
histogram={histogramData}
histoWidth={100}
histoHeight={50}
/>
</div>
);
};Setup Requirementsnpm install @tanstack/react-queryAnd then add the query client to your app: // In your app entry point
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { ReactQueryDevtools } from '@tanstack/react-query-devtools';
const queryClient = new QueryClient({
defaultOptions: {
queries: {
refetchOnWindowFocus: false,
},
},
});
function App() {
return (
<QueryClientProvider client={queryClient}>
{/* Your app components */}
<ReactQueryDevtools /> {/* Optional but helpful for debugging */}
</QueryClientProvider>
);
}Benefits of this approach:
You could extend this further by integrating with TanStack Query's query invalidation to force refreshes when needed or implement prefetching for expected user interactions. Would you like me to elaborate on any specific aspect of this implementation?✏️ Learnings added
🧠 Learnings used |
xinaesthete
force-pushed
the
deck_2025b
branch
4 times, most recently
from
de20661 to
b11dd38
Compare
xinaesthete
force-pushed
the
deck_2025b
branch
2 times, most recently
from
31a8002 to
b7c8cc3
Compare
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.
@coderabbitai I'm doing something very wrong with my state management the way
ChannelHistogramis used for colour channel limit control - I thought it was working reasonably ok at some point recently (before I went to bed), but it just goes into an infinite loop now. The original use seems ok.Summary by CodeRabbit
New Features
Refactor