Skip to content
Merged
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
2 changes: 1 addition & 1 deletion frontend/javascripts/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { createNanoEvents } from "nanoevents";

class WebknossosApplication {
// This event emitter is currently only used for two types of events:
// 1) webknossos:ready
// 1) webknossos:initialized
// When WK is done with initialization, the front-end API can be constructed.
// 2) rerender
// Most of the time, rendering happens when something in the Store changes. However,
Expand Down
1 change: 1 addition & 0 deletions frontend/javascripts/libs/window.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const _window: Window &
OlvyConfig?: ArbitraryObject | null;
managers?: Array<TextureBucketManager>;
materials?: Record<string, ShaderMaterial>;
measuredTimeToFirstRender?: boolean;
} =
typeof window === "undefined"
? ({
Expand Down
13 changes: 10 additions & 3 deletions frontend/javascripts/router/route_wrappers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,21 @@ import * as Utils from "libs/utils";
import { coalesce } from "libs/utils";
import window from "libs/window";
import { isNumber } from "lodash";
import { useEffect } from "react";
import { Navigate, useLocation, useParams } from "react-router-dom";
import { APICompoundTypeEnum, type APIMagRestrictions, TracingTypeEnum } from "types/api_types";
import { ControlModeEnum } from "viewer/constants";
import { ControlModeEnum, PerformanceMarkEnum } from "viewer/constants";
import { getDatasetIdOrNameFromReadableURLPart } from "viewer/model/accessors/dataset_accessor";
import { Store } from "viewer/singletons";
import TracingLayoutView from "viewer/view/layouting/tracing_layout_view";
import { PageNotFoundView } from "./page_not_found_view";

function markTracingViewLoadStartEffect() {
const markName = PerformanceMarkEnum.TRACING_VIEW_LOAD;
performance.mark(markName);
return () => performance.clearMarks(markName);
}

export function RootRouteWrapper() {
const isAuthenticated = useWkSelector((state) => state.activeUser != null);
const hasOrganizations = useWkSelector((state) => state.uiInformation.hasOrganizations);
Expand Down Expand Up @@ -169,7 +176,7 @@ export function ShortLinksRouteWrapper() {
export function TracingViewRouteWrapper() {
const { type, id } = useParams();
const initialMaybeCompoundType = type != null ? coalesce(APICompoundTypeEnum, type) : null;

useEffect(markTracingViewLoadStartEffect, []);
return (
<TracingLayoutView
initialMaybeCompoundType={initialMaybeCompoundType}
Expand Down Expand Up @@ -269,7 +276,7 @@ export function TracingViewModeRouteWrapper() {

const { datasetId, datasetName } = getDatasetIdOrNameFromReadableURLPart(datasetNameAndId);
const getParams = Utils.getUrlParamsObjectFromString(location.search);

useEffect(markTracingViewLoadStartEffect, []);
if (datasetName) {
// Handle very old legacy URLs which neither have a datasetId nor an organizationId.
// The schema is something like <authority>/datasets/:datasetName/view
Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/test/api/api_skeleton_latest.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function applyRotationInFlycamReducerSpace(

describe("API Skeleton", () => {
beforeEach<WebknossosTestContext>(async (context) => {
await setupWebknossosForTesting(context, "skeleton", { dontDispatchWkReady: true });
await setupWebknossosForTesting(context, "skeleton", { dontDispatchWkInitialized: true });
});

it<WebknossosTestContext>("getActiveNodeId should get the active node id", ({ api }) => {
Expand Down
16 changes: 10 additions & 6 deletions frontend/javascripts/test/helpers/apiHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ import {
sendSaveRequestWithToken,
type MinCutTargetEdge,
} from "admin/rest_api";
import { resetStoreAction, restartSagaAction, wkReadyAction } from "viewer/model/actions/actions";
import {
resetStoreAction,
restartSagaAction,
wkInitializedAction,
} from "viewer/model/actions/actions";
import { setActiveUserAction } from "viewer/model/actions/user_actions";
import {
tracings as HYBRID_TRACINGS,
Expand Down Expand Up @@ -314,7 +318,7 @@ startSaga(rootSaga);
export async function setupWebknossosForTesting(
testContext: WebknossosTestContext,
mode: keyof typeof modelData,
options?: { dontDispatchWkReady?: boolean },
options?: { dontDispatchWkInitialized?: boolean },
): Promise<void> {
/*
* This will execute model.fetch(...) and initialize the store with the tracing, etc.
Expand Down Expand Up @@ -387,17 +391,17 @@ export async function setupWebknossosForTesting(
true,
);
// Trigger the event ourselves, as the webKnossosController is not instantiated
app.vent.emit("webknossos:ready");
app.vent.emit("webknossos:initialized");

const api = await webknossos.apiReady();
testContext.api = api;

// Ensure the slow compression is disabled by default. Tests may change
// this individually.
testContext.setSlowCompression(false);
if (!options?.dontDispatchWkReady) {
// Dispatch the wkReadyAction, so the sagas are started
Store.dispatch(wkReadyAction());
if (!options?.dontDispatchWkInitialized) {
// Dispatch the wkInitializedAction, so the sagas are started
Store.dispatch(wkInitializedAction());
}
} catch (error) {
console.error("model.fetch() failed", error);
Expand Down
10 changes: 5 additions & 5 deletions frontend/javascripts/test/sagas/annotation_saga.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import {
setBlockedByUserAction,
setOthersMayEditForAnnotationAction,
} from "viewer/model/actions/annotation_actions";
import { ensureWkReady } from "viewer/model/sagas/ready_sagas";
import { wkReadyAction } from "viewer/model/actions/actions";
import { ensureWkInitialized } from "viewer/model/sagas/ready_sagas";
import { wkInitializedAction } from "viewer/model/actions/actions";
import { acquireAnnotationMutexMaybe } from "viewer/model/sagas/annotation_saga";

const createInitialState = (
Expand Down Expand Up @@ -39,7 +39,7 @@ describe("Annotation Saga", () => {
const storeState = createInitialState(false, false);
const saga = acquireAnnotationMutexMaybe();
saga.next();
saga.next(wkReadyAction());
saga.next(wkInitializedAction());
saga.next(storeState.annotation.restrictions.allowUpdate);
saga.next(storeState.annotation.annotationId);
expect(saga.next().done, "The saga should terminate.").toBe(true);
Expand All @@ -51,9 +51,9 @@ describe("Annotation Saga", () => {
const storeState = createInitialState(othersMayEdit);
const saga = acquireAnnotationMutexMaybe();

expectValueDeepEqual(expect, saga.next(), call(ensureWkReady));
expectValueDeepEqual(expect, saga.next(), call(ensureWkInitialized));
expect(
saga.next(wkReadyAction()).value.type,
saga.next(wkInitializedAction()).value.type,
"The saga should select the allowUpdate next.",
).toBe("SELECT");
expect(
Expand Down
8 changes: 4 additions & 4 deletions frontend/javascripts/test/sagas/save_saga.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { alert } from "libs/window";
import { setSaveBusyAction } from "viewer/model/actions/save_actions";
import DiffableMap from "libs/diffable_map";
import compactSaveQueue from "viewer/model/helpers/compaction/compact_save_queue";
import { ensureWkReady } from "viewer/model/sagas/ready_sagas";
import { ensureWkInitialized } from "viewer/model/sagas/ready_sagas";
import { createSaveQueueFromUpdateActions } from "../helpers/saveHelpers";
import { expectValueDeepEqual } from "../helpers/sagaHelpers";
import { UnitLong } from "viewer/constants";
Expand Down Expand Up @@ -94,7 +94,7 @@ describe("Save Saga", () => {
];
const saveQueue = createSaveQueueFromUpdateActions(updateActions, TIMESTAMP);
const saga = pushSaveQueueAsync();
expectValueDeepEqual(expect, saga.next(), call(ensureWkReady));
expectValueDeepEqual(expect, saga.next(), call(ensureWkInitialized));
saga.next(); // setLastSaveTimestampAction

saga.next(); // select state
Expand Down Expand Up @@ -248,7 +248,7 @@ describe("Save Saga", () => {
];
const saveQueue = createSaveQueueFromUpdateActions(updateActions, TIMESTAMP);
const saga = pushSaveQueueAsync();
expectValueDeepEqual(expect, saga.next(), call(ensureWkReady));
expectValueDeepEqual(expect, saga.next(), call(ensureWkInitialized));

saga.next();
saga.next(); // select state
Expand Down Expand Up @@ -276,7 +276,7 @@ describe("Save Saga", () => {
];
const saveQueue = createSaveQueueFromUpdateActions(updateActions, TIMESTAMP);
const saga = pushSaveQueueAsync();
expectValueDeepEqual(expect, saga.next(), call(ensureWkReady));
expectValueDeepEqual(expect, saga.next(), call(ensureWkInitialized));
saga.next();
saga.next(); // select state

Expand Down
4 changes: 2 additions & 2 deletions frontend/javascripts/viewer/api/api_latest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ import {
getVolumeTracings,
hasVolumeTracings,
} from "viewer/model/accessors/volumetracing_accessor";
import { restartSagaAction, wkReadyAction } from "viewer/model/actions/actions";
import { restartSagaAction, wkInitializedAction } from "viewer/model/actions/actions";
import {
dispatchMaybeFetchMeshFilesAsync,
refreshMeshesAction,
Expand Down Expand Up @@ -1161,7 +1161,7 @@ class TracingApi {
version,
);
Store.dispatch(discardSaveQueuesAction());
Store.dispatch(wkReadyAction());
Store.dispatch(wkInitializedAction());
UrlManager.updateUnthrottled();
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/viewer/api/api_loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class ApiLoader {

constructor(webKnossosModel: WebKnossosModel) {
this.readyPromise = new Promise((resolve) => {
app.vent.on("webknossos:ready", resolve);
app.vent.on("webknossos:initialized", resolve);
});
this.model = webKnossosModel;
this.DEV = new WkDev(this);
Expand Down
5 changes: 5 additions & 0 deletions frontend/javascripts/viewer/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,3 +508,8 @@ export enum AnnotationStateFilterEnum {
ACTIVE = "Active",
FINISHED_OR_ARCHIVED = "Finished",
}

export enum PerformanceMarkEnum {
TRACING_VIEW_LOAD = "tracing_view_load_start",
SHADER_COMPILE = "shader_compile_start",
}
49 changes: 33 additions & 16 deletions frontend/javascripts/viewer/controller.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import UrlManager from "viewer/controller/url_manager";
import ArbitraryController from "viewer/controller/viewmodes/arbitrary_controller";
import PlaneController from "viewer/controller/viewmodes/plane_controller";
import { AnnotationTool } from "viewer/model/accessors/tool_accessor";
import { wkReadyAction } from "viewer/model/actions/actions";
import { wkInitializedAction } from "viewer/model/actions/actions";
import { redoAction, saveNowAction, undoAction } from "viewer/model/actions/save_actions";
import { setViewModeAction, updateLayerSettingAction } from "viewer/model/actions/settings_actions";
import { setIsInAnnotationViewAction } from "viewer/model/actions/ui_actions";
Expand All @@ -42,6 +42,8 @@ type OwnProps = {
type StateProps = {
viewMode: ViewMode;
user: APIUser | null | undefined;
isUiReady: boolean;
isWkInitialized: boolean;
};
type Props = OwnProps & StateProps;
type PropsWithRouter = Props & RouteComponentProps & WithBlockerProps;
Expand Down Expand Up @@ -174,14 +176,9 @@ class Controller extends React.PureComponent<PropsWithRouter, State> {
this.initKeyboard();
this.initTaskScript();
window.webknossos = new ApiLoader(Model);
app.vent.emit("webknossos:ready");
Store.dispatch(wkReadyAction());
setTimeout(() => {
// Give wk (sagas and bucket loading) a bit time to catch air before
// showing the UI as "ready". The goal here is to avoid that the
// UI is still freezing after the loading indicator is gone.
this.props.setControllerStatus("loaded");
}, 200);
app.vent.emit("webknossos:initialized");
Store.dispatch(wkInitializedAction());
this.props.setControllerStatus("loaded");
}

async initTaskScript() {
Expand Down Expand Up @@ -309,20 +306,22 @@ class Controller extends React.PureComponent<PropsWithRouter, State> {

render() {
const status = this.props.controllerStatus;
const { user, viewMode } = this.props;
const { user, viewMode, isUiReady, isWkInitialized } = this.props;
const { gotUnhandledError, organizationToSwitchTo } = this.state;

if (status === "loading") {
return <BrainSpinner />;
let cover = null;
// Show the brain spinner during loading and until the UI is ready
if (status === "loading" || (status === "loaded" && !isUiReady)) {
cover = <BrainSpinner />;
} else if (status === "failedLoading" && user != null) {
return (
cover = (
<BrainSpinnerWithError
gotUnhandledError={gotUnhandledError}
organizationToSwitchTo={organizationToSwitchTo}
/>
);
} else if (status === "failedLoading") {
return (
Comment on lines -315 to -325
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the cause for bug noticed here: https://scm.slack.com/archives/C5AKLAV0B/p1761766742395699.

The issue seems to be that the status is still loading but wk is already initialized. In that case the previous logic just rendered the brain spinner. The new code now also tries to render the PlaneController when status === "loading" && isWkInitialized === true. And PlaneController tries to access the scene controller which is not yet re-initialized.

Reverting this code seems to fix the bug. But not sure whether this is actually hiding a bug 🤔

cover = (
<CoverWithLogin
onLoggedIn={() => {
// Close existing error toasts for "Not Found" errors before trying again.
Expand All @@ -334,6 +333,12 @@ class Controller extends React.PureComponent<PropsWithRouter, State> {
);
}

// If wk is not initialized yet, only render the cover. If it is initialized, start rendering the controllers
// in the background, hidden by the cover.
if (!isWkInitialized) {
return cover;
}

const { allowedModes } = Store.getState().annotation.restrictions;

if (!allowedModes.includes(viewMode)) {
Expand All @@ -347,9 +352,19 @@ class Controller extends React.PureComponent<PropsWithRouter, State> {
const isPlane = constants.MODES_PLANE.includes(viewMode);

if (isArbitrary) {
return <ArbitraryController viewMode={viewMode} />;
return (
<>
{cover != null ? cover : null}
<ArbitraryController viewMode={viewMode} />
</>
);
} else if (isPlane) {
return <PlaneController />;
return (
<>
{cover != null ? cover : null}
<PlaneController />
</>
);
} else {
// At the moment, all possible view modes consist of the union of MODES_ARBITRARY and MODES_PLANE
// In case we add new viewmodes, the following error will be thrown.
Expand All @@ -360,6 +375,8 @@ class Controller extends React.PureComponent<PropsWithRouter, State> {

function mapStateToProps(state: WebknossosState): StateProps {
return {
isUiReady: state.uiInformation.isUiReady,
isWkInitialized: state.uiInformation.isWkInitialized,
viewMode: state.temporaryConfiguration.viewMode,
user: state.activeUser,
};
Expand Down
25 changes: 10 additions & 15 deletions frontend/javascripts/viewer/controller/camera_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,21 +111,16 @@ class CameraController extends React.PureComponent<Props> {
const tdId = `inputcatcher_${OrthoViews.TDView}`;
this.bindToEvents();
Utils.waitForElementWithId(tdId).then(() => {
// Without this setTimeout, the initial camera angle/position
// within the 3D viewport is incorrect.
// Also see #5455.
setTimeout(() => {
this.props.setTargetAndFixPosition();
Store.dispatch(
setTDCameraWithoutTimeTrackingAction({
near: 0,
far,
}),
);
api.tracing.rotate3DViewToDiagonal(false);
const tdData = Store.getState().viewModeData.plane.tdCamera;
this.updateTDCamera(tdData);
}, 0);
this.props.setTargetAndFixPosition();
Store.dispatch(
setTDCameraWithoutTimeTrackingAction({
near: 0,
far,
}),
);
api.tracing.rotate3DViewToDiagonal(false);
const tdData = Store.getState().viewModeData.plane.tdCamera;
this.updateTDCamera(tdData);
});
}

Expand Down
Loading