Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve shape metadata update logic #385

Merged
merged 4 commits into from
Jul 30, 2024
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
12 changes: 0 additions & 12 deletions packages/base/src/3dview/mainviewmodel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { IWorkerMessage } from '@jupytercad/occ-worker';
import {
IAnnotation,
IDict,
IDisplayShape,
IJcadObjectDocChange,
IJCadWorker,
IJCadWorkerRegistry,
Expand Down Expand Up @@ -106,8 +105,6 @@ export class MainViewModel implements IDisposable {
}
});

this._saveMeta(result);

if (this._firstRender) {
const postShapes = this._jcadModel.sharedModel
.outputs as any as IDict<IPostResult>;
Expand Down Expand Up @@ -231,15 +228,6 @@ export class MainViewModel implements IDisposable {
}
}

private _saveMeta(payload: IDisplayShape['payload']['result']) {
if (!this._jcadModel) {
return;
}
Object.entries(payload).forEach(([objName, data]) => {
this._jcadModel.sharedModel.setShapeMeta(objName, data.meta);
});
}

private async _onSharedObjectsChanged(
_: IJupyterCadDoc,
change: IJcadObjectDocChange
Expand Down
60 changes: 51 additions & 9 deletions packages/base/src/commands.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
IDict,
IDryRunResponsePayload,
IJCadContent,
IJCadFormSchemaRegistry,
IJCadObject,
Expand Down Expand Up @@ -38,6 +39,7 @@ import keybindings from './keybindings.json';
import { JupyterCadPanel, JupyterCadWidget } from './widget';
import { DocumentRegistry } from '@jupyterlab/docregistry';
import { PathExt } from '@jupyterlab/coreutils';
import { MainViewModel } from './3dview/mainviewmodel';
import { handleRemoveObject } from './panelview';

export function newName(type: string, model: IJupyterCadModel): string {
Expand All @@ -52,6 +54,23 @@ export function newName(type: string, model: IJupyterCadModel): string {
return name;
}

export async function dryRunCheck(options: {
trungleduc marked this conversation as resolved.
Show resolved Hide resolved
jcadContent: IJCadContent;
mainView: MainViewModel;
requestedOperator: string;
}): Promise<IDryRunResponsePayload | null> {
const { jcadContent, mainView, requestedOperator } = options;
const dryRunResult = await mainView.dryRun(jcadContent);
if (dryRunResult.status === 'error') {
showErrorMessage(
`Failed to apply ${requestedOperator} operator`,
`The ${requestedOperator} tool was unable to create the desired shape due to invalid parameter values. The values you entered may not be compatible with the dimensions of your piece.`
);
return null;
}
return dryRunResult;
}

export function setVisible(
sharedModel: IJupyterCadDoc,
name: string,
Expand Down Expand Up @@ -208,18 +227,21 @@ export async function executeOperator(
...currentJcadContent,
objects: [...currentJcadContent.objects, objectModel]
};
const dryRunResult =
await current.content.currentViewModel.dryRun(updatedContent);
if (dryRunResult.status === 'error') {
showErrorMessage(
`Failed to create the ${name} operation`,
`The ${name} tool was unable to create the desired shape due to invalid parameter values. The values you entered may not be compatible with the dimensions of your piece.`
);

const dryRunResult = await dryRunCheck({
jcadContent: updatedContent,
mainView: current.content.currentViewModel,
requestedOperator: name
});
if (!dryRunResult) {
return;
trungleduc marked this conversation as resolved.
Show resolved Hide resolved
}

// Everything's good, we can apply the change to the shared model

const objMeta = dryRunResult.shapeMetadata?.[objectModel.name];
if (objMeta) {
objectModel.shapeMetadata = objMeta;
}
sharedModel.transact(() => {
transaction(sharedModel);
});
Expand Down Expand Up @@ -1093,7 +1115,7 @@ namespace Private {
title: value.title,
sourceData: value.default(current.context.model),
schema: FORM_SCHEMA[value.shape],
syncData: (props: IDict) => {
syncData: async (props: IDict) => {
const { Name, ...parameters } = props;
const objectModel: IJCadObject = {
shape: value.shape as Parts,
Expand All @@ -1103,8 +1125,28 @@ namespace Private {
};

const sharedModel = current.context.model.sharedModel;

if (sharedModel) {
if (!sharedModel.objectExists(objectModel.name)) {
// Try a dry run with the update content to verify its feasibility
const currentJcadContent = current.context.model.getContent();
const updatedContent: IJCadContent = {
...currentJcadContent,
objects: [...currentJcadContent.objects, objectModel]
};

const dryRunResult = await dryRunCheck({
jcadContent: updatedContent,
mainView: current.content.currentViewModel,
requestedOperator: value.shape
});
if (!dryRunResult) {
return;
}
const objMeta = dryRunResult.shapeMetadata?.[objectModel.name];
if (objMeta) {
objectModel.shapeMetadata = objMeta;
}
sharedModel.addObject(objectModel);
} else {
showErrorMessage(
Expand Down
13 changes: 10 additions & 3 deletions packages/base/src/panelview/objectproperties.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,18 @@ class ObjectPropertiesReact extends React.Component<IProps, IStates> {
}

// Dry run was successful, ready to apply the update now
const meta: IDict = dryRunResult.shapeMetadata?.[objectName] ?? {};
const obj = model.sharedModel.getObjectByName(objectName);
if (obj) {
model.sharedModel.updateObjectByName(objectName, 'parameters', {
...obj['parameters'],
...properties
model.sharedModel.updateObjectByName(objectName, {
data: {
key: 'parameters',
value: {
...obj['parameters'],
...properties
}
},
meta
});
}
}
Expand Down
11 changes: 8 additions & 3 deletions packages/occ-worker/src/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ self.onmessage = async (event: MessageEvent): Promise<void> => {
break;
}
case WorkerAction.DRY_RUN: {
let response: IDict = {};
try {
WorkerHandler[WorkerAction.DRY_RUN](message.payload);
response = WorkerHandler[WorkerAction.DRY_RUN](message.payload);
} catch (e) {
let msg = '';

Expand All @@ -80,13 +81,17 @@ self.onmessage = async (event: MessageEvent): Promise<void> => {
);
return;
}

const shapeMetadata: IDict = {};
Object.entries(response.result ?? {}).forEach(([objName, data]) => {
shapeMetadata[objName] = (data as any)?.['meta'];
});
sendToMain(
{
action: MainAction.DRY_RUN_RESPONSE,
payload: {
id: message.payload.id,
status: 'ok'
status: 'ok',
shapeMetadata
}
},
id
Expand Down
10 changes: 8 additions & 2 deletions packages/schema/src/doc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,18 @@ export class JupyterCadDoc
});
}

updateObjectByName(name: string, key: string, value: any): void {
updateObjectByName(
name: string,
payload: { data: { key: string; value: any }; meta?: IDict }
): void {
const obj = this._getObjectAsYMapByName(name);
if (!obj) {
return;
}
const { key, value } = payload.data;

this.transact(() => {
// Special case for changing parameters, we may need to update dependencies
console.log('update ', key);
if (key === 'parameters') {
switch (obj.get('shape')) {
case 'Part::Cut': {
Expand All @@ -190,6 +193,9 @@ export class JupyterCadDoc
}

obj.set(key, value);
if (payload.meta) {
obj.set('shapeMetadata', payload.meta);
}
});
}

Expand Down
6 changes: 5 additions & 1 deletion packages/schema/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ export interface IJupyterCadDoc extends YDocument<IJupyterCadDocChange> {
removeObjectByName(name: string): void;
addObject(value: IJCadObject): void;
addObjects(value: Array<IJCadObject>): void;
updateObjectByName(name: string, key: string, value: any): void;
updateObjectByName(
name: string,
payload: { data: { key: string; value: any }; meta?: IDict }
): void;
getDependants(name: string): string[];

getOption(key: keyof IJCadOptions): IDict | undefined;
Expand Down Expand Up @@ -258,6 +261,7 @@ export interface IDryRunResponsePayload {
id: string;
status: 'ok' | 'error';
message?: string;
shapeMetadata?: IDict;
}

export interface IDryRunResponse extends IMainMessageBase {
Expand Down
1 change: 1 addition & 0 deletions python/jupytercad_core/jupytercad_core/jcad_ydoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def get(self) -> str:
return json.dumps(
dict(objects=objects, options=options, metadata=meta, outputs=outputs),
indent=2,
sort_keys=True,
)

def set(self, value: str) -> None:
Expand Down
60 changes: 60 additions & 0 deletions ui-tests/tests/ui.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,4 +297,64 @@ test.describe('UI Test', () => {
}
});
});

test.describe('JCAD creation test', () => {
test.describe('Extension activation test', () => {
test('should create a new JCAD file', async ({ page, request }) => {
await page.goto();
await page
.getByLabel('notebook content')
.getByText('New JCAD File')
.click();

await page.getByTitle('New Box').getByRole('button').click();
await page.getByRole('button', { name: 'Submit' }).click();
await page.waitForTimeout(1000);
await page.getByText('Box 1').click();
await page.locator('#tab-key-1-6').click();

await page.waitForTimeout(1000);
const main = await page.locator('#jp-main-dock-panel');

if (main) {
expect(await main.screenshot()).toMatchSnapshot({
name: `JCAD-New.png`
});
}
await page.getByLabel('Length*').fill('0.5');
await page.getByRole('button', { name: 'Submit' }).click();
await page.waitForTimeout(500);
await page.getByLabel('Width*').fill('1.5');
await page.getByRole('button', { name: 'Submit' }).click();
await page.waitForTimeout(500);
await page.getByLabel('Height*').fill('2');
await page.getByRole('button', { name: 'Submit' }).click();
await page.waitForTimeout(500);
if (main) {
expect(await main.screenshot()).toMatchSnapshot({
name: `JCAD-Modified.png`
});
}

await page.getByTitle('Undo').getByRole('button').click();
await page.getByTitle('Undo').getByRole('button').click();
await page.getByTitle('Undo').getByRole('button').click();
await page.waitForTimeout(1000);
if (main) {
expect(await main.screenshot()).toMatchSnapshot({
name: `JCAD-Undo.png`
});
}
await page.getByTitle('Redo').getByRole('button').click();
await page.getByTitle('Redo').getByRole('button').click();
await page.getByTitle('Redo').getByRole('button').click();
await page.waitForTimeout(1000);
if (main) {
expect(await main.screenshot()).toMatchSnapshot({
name: `JCAD-Redo.png`
});
}
});
});
});
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading