Skip to content

Commit

Permalink
Including missing improvement that was available on the original proj…
Browse files Browse the repository at this point in the history
…ect (#29)

**User-Facing Changes**
That are some perfomance issues in the 3D panel rendering and we would
like to include it in the aplication.

**Description**

I've added [ this
commit](foxglove/studio@ed3946c)
from the original project in order to optimize the 3D rendering.
  • Loading branch information
laisspportugal authored May 24, 2024
1 parent 28d5647 commit 7a4e869
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 23 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "foxbox",
"version": "1.0.2",
"version": "1.0.3",
"license": "MPL-2.0",
"private": true,
"productName": "Foxbox",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

/* eslint-disable jest/no-conditional-expect */

import { renderHook, act } from "@testing-library/react";
import { act, renderHook } from "@testing-library/react";
import { PropsWithChildren, useCallback, useState } from "react";
import { DeepPartial } from "ts-essentials";

Expand All @@ -29,7 +29,7 @@ import MockCurrentLayoutProvider from "@foxglove/studio-base/providers/CurrentLa
import delay from "@foxglove/studio-base/util/delay";
import { makeMockAppConfiguration } from "@foxglove/studio-base/util/makeMockAppConfiguration";

import { MessagePipelineProvider, useMessagePipeline, MessagePipelineContext } from ".";
import { MessagePipelineContext, MessagePipelineProvider, useMessagePipeline } from ".";
import FakePlayer from "./FakePlayer";
import { MAX_PROMISE_TIMEOUT_TIME_MS } from "./pauseFrameForPromise";

Expand Down Expand Up @@ -136,7 +136,7 @@ describe("MessagePipelineProvider/useMessagePipeline", () => {
playerState: {
activeData: undefined,
capabilities: [],
presence: PlayerPresence.NOT_PRESENT,
presence: PlayerPresence.INITIALIZING,
playerId: "",
progress: {},
},
Expand Down Expand Up @@ -199,7 +199,7 @@ describe("MessagePipelineProvider/useMessagePipeline", () => {
expect(all[0]!.playerState).toEqual({
activeData: undefined,
capabilities: [],
presence: PlayerPresence.NOT_PRESENT,
presence: PlayerPresence.INITIALIZING,
playerId: "",
progress: {},
});
Expand Down Expand Up @@ -259,7 +259,7 @@ describe("MessagePipelineProvider/useMessagePipeline", () => {
playerState: {
activeData: undefined,
capabilities: [],
presence: PlayerPresence.NOT_PRESENT,
presence: PlayerPresence.INITIALIZING,
playerId: "",
progress: {},
},
Expand Down
14 changes: 5 additions & 9 deletions packages/studio-base/src/components/MessagePipeline/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,16 @@ import {
PlayerState,
SubscribePayload,
} from "@foxglove/studio-base/players/types";
import { assertNever } from "@foxglove/studio-base/util/assertNever";
import isDesktopApp from "@foxglove/studio-base/util/isDesktopApp";

import { FramePromise } from "./pauseFrameForPromise";
import { MessagePipelineContext } from "./types";

export function defaultPlayerState(): PlayerState {
export function defaultPlayerState(player?: Player): PlayerState {
return {
presence: PlayerPresence.NOT_PRESENT,
// when there is a player we default to initializing, to prevent thrashing in the UI when
// the player is initialized.
presence: player ? PlayerPresence.INITIALIZING : PlayerPresence.NOT_PRESENT,
progress: {},
capabilities: [],
profile: undefined,
Expand Down Expand Up @@ -138,7 +139,7 @@ export function createMessagePipelineStore({
},

public: {
playerState: defaultPlayerState(),
playerState: defaultPlayerState(initialPlayer),
messageEventsBySubscriberId: new Map(),
subscriptions: [],
sortedTopics: [],
Expand Down Expand Up @@ -423,11 +424,6 @@ export function reducer(
};
}
}

assertNever(
action,
`Unhandled message pipeline action type ${(action as MessagePipelineStateAction).type}`,
);
}

async function builtinFetch(url: string, opts?: { signal?: AbortSignal }) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { CSSProperties, useEffect, useLayoutEffect, useMemo, useRef, useState }
import { useLatest } from "react-use";
import { v4 as uuid } from "uuid";

import { useValueChangedDebugLog, useSynchronousMountedState } from "@foxglove/hooks";
import { useSynchronousMountedState, useValueChangedDebugLog } from "@foxglove/hooks";
import Logger from "@foxglove/log";
import { fromSec, toSec } from "@foxglove/rostime";
import {
Expand Down Expand Up @@ -42,11 +42,12 @@ import useGlobalVariables from "@foxglove/studio-base/hooks/useGlobalVariables";
import {
AdvertiseOptions,
PlayerCapabilities,
PlayerPresence,
SubscribePayload,
} from "@foxglove/studio-base/players/types";
import {
usePanelSettingsTreeUpdate,
useDefaultPanelTitle,
usePanelSettingsTreeUpdate,
} from "@foxglove/studio-base/providers/PanelStateContextProvider";
import { PanelConfig, SaveConfig } from "@foxglove/studio-base/types/panels";
import { assertNever } from "@foxglove/studio-base/util/assertNever";
Expand Down Expand Up @@ -118,7 +119,7 @@ function PanelExtensionAdapter(
const { playerState, pauseFrame, setSubscriptions, seekPlayback, sortedTopics } =
messagePipelineContext;

const { capabilities, profile: dataSourceProfile } = playerState;
const { capabilities, profile: dataSourceProfile, presence: playerPresence } = playerState;

const { openSiblingPanel, setMessagePathDropConfig } = usePanelContext();

Expand Down Expand Up @@ -547,6 +548,8 @@ function PanelExtensionAdapter(
);
}, [initialState, highestSupportedConfigVersion]);

const playerIsInitializing = playerPresence === PlayerPresence.INITIALIZING;

// Manage extension lifecycle by calling initPanel() when the panel context changes.
//
// If we useEffect here instead of useLayoutEffect, the prevRenderState can get polluted with data
Expand All @@ -556,9 +559,12 @@ function PanelExtensionAdapter(
throw new Error("Expected panel container to be mounted");
}

// If the config is too new for this panel to support we bail and don't do any panel initialization
// We will instead show a warning message to the user
if (configTooNew) {
// Also don't show panel when the player is initializing. The initializing state is temporary for
// players to go through to load their sources. Once a player has completed initialization `initPanel` is called again (or even a few times),
// because parts of the player context have changed. This cleans up the old panel that was present
// during initialization. So there can be no state held between extension panels between initialization and
// whatever follows it. To prevent this unnecessary render, we do not render the panel during initialization.
if (configTooNew || playerIsInitializing) {
return;
}

Expand Down Expand Up @@ -596,7 +602,14 @@ function PanelExtensionAdapter(
getMessagePipelineContext().setSubscriptions(panelId, []);
getMessagePipelineContext().setPublishers(panelId, []);
};
}, [initPanel, panelId, partialExtensionContext, getMessagePipelineContext, configTooNew]);
}, [
initPanel,
panelId,
partialExtensionContext,
getMessagePipelineContext,
configTooNew,
playerIsInitializing,
]);

const style: CSSProperties = {};
if (slowRender) {
Expand Down
2 changes: 1 addition & 1 deletion packages/studio/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@foxglove/studio",
"version": "1.0.2",
"version": "1.0.3",
"license": "MPL-2.0",
"repository": {
"type": "git",
Expand Down

0 comments on commit 7a4e869

Please sign in to comment.