Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 51 additions & 24 deletions packages/suite-base/src/components/Panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,56 @@ export default function Panel<
type,
]);

const handleSetShowLogs = useCallback(({ show }: { show: boolean }) => {
setShowLogs(show);
}, []);

const panelContextValue = useMemo(
() => ({
type,
id: childId,
title,
config: panelComponentConfig,
saveConfig: saveConfig as SaveConfig<PanelConfig>,
updatePanelConfigs,
openSiblingPanel,
replacePanel,
enterFullscreen,
exitFullscreen,
setHasFullscreenDescendant,
isFullscreen: fullscreen,
tabId,
connectToolbarDragHandle: isTopLevelPanel ? undefined : connectToolbarDragHandle,
setMessagePathDropConfig,
showLogs,
setShowLogs: handleSetShowLogs,
logError: addLog,
logCount: logs.length,
}),
[
type,
childId,
title,
panelComponentConfig,
saveConfig,
updatePanelConfigs,
openSiblingPanel,
replacePanel,
enterFullscreen,
exitFullscreen,
setHasFullscreenDescendant,
fullscreen,
tabId,
isTopLevelPanel,
connectToolbarDragHandle,
setMessagePathDropConfig,
showLogs,
handleSetShowLogs,
addLog,
logs.length,
],
);

return (
<Profiler
id={childId}
Expand All @@ -563,30 +613,7 @@ export default function Panel<
}}
>
<PanelContext.Provider
value={{
type,
id: childId,
title,
config: panelComponentConfig,
saveConfig: saveConfig as SaveConfig<PanelConfig>,
updatePanelConfigs,
openSiblingPanel,
replacePanel,
enterFullscreen,
exitFullscreen,
setHasFullscreenDescendant,
isFullscreen: fullscreen,
tabId,
// disallow dragging the root panel in a layout
connectToolbarDragHandle: isTopLevelPanel ? undefined : connectToolbarDragHandle,
setMessagePathDropConfig,
showLogs,
setShowLogs: ({ show }) => {
setShowLogs(show);
},
logError: addLog,
logCount: logs.length,
}}
value={panelContextValue}
>
<KeyListener global keyUpHandlers={keyUpHandlers} keyDownHandlers={keyDownHandlers} />
{fullscreen && <KeyListener global keyDownHandlers={fullScreenKeyHandlers} />}
Expand Down
88 changes: 61 additions & 27 deletions packages/suite-base/src/panels/ThreeDeeRender/ThreeDeeRender.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,22 @@ export function ThreeDeeRender(props: Readonly<ThreeDeeRenderProps>): React.JSX.
const [loadedTransformCount, setLoadedTransformCount] = useState<number>(0);
const [reloadPreloadTrigger, setReloadPreloadTrigger] = useState<number>(0);

const renderRef = useRef({ needsRender: false });
// Incremented whenever a WebGL frame needs to be rendered.
// Using a counter (vs. a boolean ref) makes the dependency trackable by React's effect system,
// so the animationFrame() call only fires when genuinely needed rather than after every commit.
const [renderToken, setRenderToken] = useState(0);
const requestRender = useCallback(() => { setRenderToken((t) => t + 1); }, []);
const [renderDone, setRenderDone] = useState<(() => void) | undefined>();

// Refs for values that are set inside onRender to avoid redundant setState calls.
// onRender is called by the pipeline on every tick; guarding with previous-value refs
// prevents unnecessary re-renders for slowly changing values.
const prevColorSchemeRef = useRef<typeof renderDone>(undefined);
const prevTopicsRef = useRef<Immutable<RenderState>["topics"]>(undefined);
const prevParametersRef = useRef<Immutable<RenderState>["parameters"]>(undefined);
const prevSharedPanelStateRef = useRef<Immutable<RenderState>["sharedPanelState"]>(undefined);
const prevTimezoneRef = useRef<string | undefined>(undefined);

const schemaSubscriptions = useRendererProperty(
"schemaSubscriptions",
"schemaSubscriptionsChanged",
Expand Down Expand Up @@ -345,24 +358,24 @@ export function ThreeDeeRender(props: Readonly<ThreeDeeRenderProps>): React.JSX.
useEffect(() => {
if (renderer) {
renderer.config = config;
renderRef.current.needsRender = true;
requestRender();
}
}, [config, renderer]);
}, [config, renderer, requestRender]);

// Update the renderer's reference to `topics` when it changes
useEffect(() => {
if (renderer) {
renderer.setTopics(topics);
renderRef.current.needsRender = true;
requestRender();
}
}, [topics, renderer]);
}, [topics, renderer, requestRender]);

// Tell the renderer if we are connected to a ROS data source
useEffect(() => {
if (renderer) {
renderer.ros = context.dataSourceProfile === "ros1" || context.dataSourceProfile === "ros2";
}
}, [context.dataSourceProfile, renderer]);
}, [context.dataSourceProfile, renderer, requestRender]);

// Save panel settings whenever they change
const throttledSave = useDebouncedCallback(
Expand Down Expand Up @@ -530,21 +543,41 @@ export function ThreeDeeRender(props: Readonly<ThreeDeeRenderProps>): React.JSX.
// Set the done callback into a state variable to trigger a re-render
setRenderDone(() => done);

// Keep UI elements and the renderer aware of the current color scheme
setColorScheme(renderState.colorScheme);
// Guard slowly-changing values with previous-value refs to avoid unnecessary re-renders.
// colorScheme, topics, parameters, and sharedPanelState rarely change; calling setState
// unconditionally on each pipeline tick was causing the 3D panel to re-render at full
// playback rate even when none of these values had changed.
if (!Object.is(renderState.colorScheme, prevColorSchemeRef.current)) {
prevColorSchemeRef.current = renderState.colorScheme as unknown as typeof renderDone;
setColorScheme(renderState.colorScheme);
}

if (renderState.appSettings) {
const tz = renderState.appSettings.get(AppSetting.TIMEZONE);
setTimezone(typeof tz === "string" ? tz : undefined);
const nextTz = typeof tz === "string" ? tz : undefined;
if (nextTz !== prevTimezoneRef.current) {
prevTimezoneRef.current = nextTz;
setTimezone(nextTz);
}
}

// We may have new topics - since we are also watching for messages in
// the current frame, topics may not have changed
setTopics(renderState.topics);
if (!Object.is(renderState.topics, prevTopicsRef.current)) {
prevTopicsRef.current = renderState.topics;
setTopics(renderState.topics);
}

setSharedPanelState(renderState.sharedPanelState as Shared3DPanelState);
if (!Object.is(renderState.sharedPanelState, prevSharedPanelStateRef.current)) {
prevSharedPanelStateRef.current = renderState.sharedPanelState;
setSharedPanelState(renderState.sharedPanelState as Shared3DPanelState);
}

// Watch for any changes in the map of observed parameters
setParameters(renderState.parameters);
if (!Object.is(renderState.parameters, prevParametersRef.current)) {
prevParametersRef.current = renderState.parameters;
setParameters(renderState.parameters);
}

// currentFrame has messages on subscribed topics since the last render call
setCurrentFrameMessages(renderState.currentFrame);
Expand Down Expand Up @@ -669,9 +702,9 @@ export function ThreeDeeRender(props: Readonly<ThreeDeeRenderProps>): React.JSX.
useEffect(() => {
if (colorScheme && renderer) {
renderer.setColorScheme(colorScheme, backgroundColor);
renderRef.current.needsRender = true;
requestRender();
}
}, [backgroundColor, colorScheme, renderer]);
}, [backgroundColor, colorScheme, renderer, requestRender]);

// Handle preloaded messages and render a frame if new messages are available
// Should be called before `messages` is handled
Expand All @@ -682,9 +715,9 @@ export function ThreeDeeRender(props: Readonly<ThreeDeeRenderProps>): React.JSX.
}
const newMessagesHandled = renderer.handleAllFramesMessages(allFrames);
if (newMessagesHandled) {
renderRef.current.needsRender = true;
requestRender();
}
}, [renderer, currentTime, allFrames]);
}, [renderer, currentTime, allFrames, requestRender]);

// Handle messages and render a frame if new messages are available
useEffect(() => {
Expand All @@ -696,16 +729,16 @@ export function ThreeDeeRender(props: Readonly<ThreeDeeRenderProps>): React.JSX.
renderer.addMessageEvent(message);
}

renderRef.current.needsRender = true;
}, [currentFrameMessages, renderer]);
requestRender();
}, [currentFrameMessages, renderer, requestRender]);

// Update the renderer when the camera moves
useEffect(() => {
if (!_.isEqual(cameraState, renderer?.getCameraState())) {
renderer?.setCameraState(cameraState);
renderRef.current.needsRender = true;
requestRender();
}
}, [cameraState, renderer]);
}, [cameraState, renderer, requestRender]);

// Sync camera with shared state, if enabled.
useEffect(() => {
Expand All @@ -724,7 +757,7 @@ export function ThreeDeeRender(props: Readonly<ThreeDeeRenderProps>): React.JSX.
} else {
const newCameraState = sharedPanelState.cameraState;
renderer.setCameraState(newCameraState);
renderRef.current.needsRender = true;
requestRender();
setConfig((prevConfig) => ({
...prevConfig,
cameraState: newCameraState,
Expand All @@ -737,15 +770,16 @@ export function ThreeDeeRender(props: Readonly<ThreeDeeRenderProps>): React.JSX.
renderer,
renderer?.followFrameId,
sharedPanelState,
requestRender
]);

// Render a new frame if requested
// Render a new frame whenever renderToken is incremented.
// Using a state-based token instead of a ref means this effect only fires when a render
// is explicitly requested, not after every React commit.
useEffect(() => {
if (renderer && renderRef.current.needsRender) {
renderer.animationFrame();
renderRef.current.needsRender = false;
}
});
renderer?.animationFrame();

}, [renderer, renderToken]);

// Invoke the done callback once the render is complete
useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,10 @@ describe("CachingIterableSource", () => {
}
});

it("should clear the cache when topics change", async () => {
it("should preserve cache blocks behind the high-water mark when a topic is added", async () => {
// C-1 fix: when a new topic is added, only future blocks (ahead of the high-water mark) are
// evicted. Blocks the user has already consumed are kept so we don't lose the entire
// 600 MB cache every time a panel is opened.
const source = new TestSource();
const bufferedSource = new CachingIterableSource(source, {
maxBlockSize: 1000,
Expand Down Expand Up @@ -801,13 +804,20 @@ describe("CachingIterableSource", () => {
}

{
// Add topic "b" — this is a new topic addition (not a removal).
// The selective-eviction strategy keeps all blocks that are behind the high-water mark
// (the furthest point we've consumed). Since we read the entire recording above,
// the high-water mark is at the end, and the cached block starts at t=0 which is
// before the high-water mark — so the block is KEPT.
const messageIterator = bufferedSource.messageIterator({
topics: mockTopicSelection("a", "b"),
});

await messageIterator.next();

expect(bufferedSource.loadedRanges()).toEqual([{ start: 0, end: 0 }]);
// The loaded range must be preserved: blocks behind the high-water mark survive.
// (Old behaviour was [{start:0,end:0}] — a full wipe; new behaviour keeps the cache.)
expect(bufferedSource.loadedRanges()).toEqual([{ start: 0, end: 1 }]);
}
});

Expand Down
Loading