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.

Sign up for GitHub

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

delay updating renderers until the end of update sequences #1995

Merged
merged 1 commit into from
Mar 20, 2023
Merged

delay updating renderers until the end of update sequences #1995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 13 additions & 13 deletions cypress/e2e/DoenetML/dynamicalsystem/cobwebpolyline.cy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 0: [1, 0] },
sourceInformation: { vertex: 0 }
sourceDetails: { vertex: 0 }
}
})

Expand Down Expand Up @@ -163,7 +163,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 1: [3, 4] },
sourceInformation: { vertex: 1 }
sourceDetails: { vertex: 1 }
}
})
})
Expand Down Expand Up @@ -207,7 +207,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 1: [1, 1] },
sourceInformation: { vertex: 1 }
sourceDetails: { vertex: 1 }
}
})
})
Expand Down Expand Up @@ -248,7 +248,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 1: [1, 1.6] },
sourceInformation: { vertex: 1 }
sourceDetails: { vertex: 1 }
}
})
})
Expand Down Expand Up @@ -336,7 +336,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 2: [1.6, 1.6] },
sourceInformation: { vertex: 2 }
sourceDetails: { vertex: 2 }
}
})
})
Expand Down Expand Up @@ -392,7 +392,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 3: [1, 2] },
sourceInformation: { vertex: 3 }
sourceDetails: { vertex: 3 }
}
})
})
Expand Down Expand Up @@ -454,7 +454,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 3: [1.6, 2.4] },
sourceInformation: { vertex: 3 }
sourceDetails: { vertex: 3 }
}
})
})
Expand Down Expand Up @@ -611,7 +611,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 4: [2.4, 2.4] },
sourceInformation: { vertex: 4 }
sourceDetails: { vertex: 4 }
}
})
})
Expand Down Expand Up @@ -680,7 +680,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 5: [-1, 3] },
sourceInformation: { vertex: 5 }
sourceDetails: { vertex: 5 }
}
})
})
Expand Down Expand Up @@ -756,7 +756,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 5: [2.4, 3] },
sourceInformation: { vertex: 5 }
sourceDetails: { vertex: 5 }
}
})
})
Expand Down Expand Up @@ -832,7 +832,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 6: [3, 1] },
sourceInformation: { vertex: 6 }
sourceDetails: { vertex: 6 }
}
})
})
Expand Down Expand Up @@ -914,7 +914,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 6: [3, 3] },
sourceInformation: { vertex: 6 }
sourceDetails: { vertex: 6 }
}
})
})
Expand Down Expand Up @@ -996,7 +996,7 @@ describe('CobwebPolyline Tag Tests', function () {
componentName: "/graph1/cobweb",
args: {
pointCoords: { 7: [3, 3] },
sourceInformation: { vertex: 7 }
sourceDetails: { vertex: 7 }
}
})
})
Expand Down
110 changes: 67 additions & 43 deletions src/Core/Core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ export default class Core {
}


await this.processStateVariableTriggers();
await this.processStateVariableTriggers(true);

} else {
if (parent === undefined) {
Expand Down Expand Up @@ -589,7 +589,7 @@ export default class Core {
});
}

await this.processStateVariableTriggers();
await this.processStateVariableTriggers(true);

}

Expand Down Expand Up @@ -903,12 +903,14 @@ export default class Core {
return deletedComponentNames;
}

async processStateVariableTriggers() {
async processStateVariableTriggers(updateRenderersIfTriggered = false) {

// TODO: can we make this more efficient by only checking components that changed?
// componentsToUpdateRenderers is close, but it includes only rendered components
// and we could have components with triggers that are not rendered

let triggeredAction = false;

for (let componentName in this.stateVariableChangeTriggers) {
let component = this._components[componentName];
for (let stateVariable in this.stateVariableChangeTriggers[componentName]) {
Expand All @@ -926,15 +928,21 @@ export default class Core {
actionName: triggerInstructions.action,
args: {
stateValues: { [stateVariable]: value },
previousValues: { [stateVariable]: previousValue }
}
previousValues: { [stateVariable]: previousValue },
skipRendererUpdate: true,
},
})
triggeredAction = true;
}
}

}
}

if (triggeredAction && updateRenderersIfTriggered) {
this.updateAllChangedRenderers();
}

}

async expandAllComposites(component, force = false) {
Expand Down Expand Up @@ -8239,7 +8247,7 @@ export default class Core {
}
}

async triggerChainedActions({ componentName, triggeringAction }) {
async triggerChainedActions({ componentName, triggeringAction, actionId, sourceInformation = {}, skipRendererUpdate = false }) {

for (let cName in this.updateInfo.componentsToUpdateActionChaining) {
await this.checkForActionChaining({
Expand All @@ -8257,9 +8265,20 @@ export default class Core {

if (this.actionsChangedToActions[id]) {
for (let chainedActionInstructions of this.actionsChangedToActions[id]) {
chainedActionInstructions = { ...chainedActionInstructions };
if (chainedActionInstructions.args) {
chainedActionInstructions.args = { ...chainedActionInstructions.args }
} else {
chainedActionInstructions.args = {};
}
chainedActionInstructions.args.skipRendererUpdate = true;
await this.performAction(chainedActionInstructions);
}
}

if (!skipRendererUpdate) {
await this.updateAllChangedRenderers(sourceInformation, actionId);
}
}


Expand All @@ -8282,8 +8301,8 @@ export default class Core {
componentSourceInformation = sourceInformation[instruction.componentName] = {};
}

if (instruction.sourceInformation) {
Object.assign(componentSourceInformation, instruction.sourceInformation);
if (instruction.sourceDetails) {
Object.assign(componentSourceInformation, instruction.sourceDetails);
}
}

Expand Down Expand Up @@ -8337,13 +8356,13 @@ export default class Core {

async performUpdate({ updateInstructions, actionId, event, overrideReadOnly = false,
doNotSave = false, canSkipUpdatingRenderer = false,
skipRendererUpdate = false, sourceInformation = {},
suppressToast = false // temporary
}) {

if (this.flags.readOnly && !overrideReadOnly) {

if (!canSkipUpdatingRenderer) {
let sourceInformation = {};

for (let instruction of updateInstructions) {

Expand All @@ -8352,8 +8371,8 @@ export default class Core {
componentSourceInformation = sourceInformation[instruction.componentName] = {};
}

if (instruction.sourceInformation) {
Object.assign(componentSourceInformation, instruction.sourceInformation);
if (instruction.sourceDetails) {
Object.assign(componentSourceInformation, instruction.sourceDetails);
}
}

Expand All @@ -8370,7 +8389,6 @@ export default class Core {

let newStateVariableValues = {};
let newStateVariableValuesProcessed = [];
let sourceInformation = {};
let workspace = {};
let recordItemSubmissions = [];

Expand All @@ -8382,8 +8400,8 @@ export default class Core {
componentSourceInformation = sourceInformation[instruction.componentName] = {};
}

if (instruction.sourceInformation) {
Object.assign(componentSourceInformation, instruction.sourceInformation);
if (instruction.sourceDetails) {
Object.assign(componentSourceInformation, instruction.sourceDetails);
}
}

Expand Down Expand Up @@ -8449,37 +8467,12 @@ export default class Core {
updateInstructions.forEach(comp => { if (comp.componentName) { this.updateInfo.componentsToUpdateRenderers.add(comp.componentName) } });
}

let componentNamesToUpdate = [...this.updateInfo.componentsToUpdateRenderers];
this.updateInfo.componentsToUpdateRenderers.clear();

await this.updateRendererInstructions({
componentNamesToUpdate,
sourceOfUpdate: { sourceInformation, local: true },
actionId,
});



// updating renderer instructions could trigger more composite updates
// (presumably from deriving child results)
// if so, make replacement changes and update renderer instructions again
// TODO: should we check for child results earlier so we don't have to check them
// when updating renderer instructions?
if (this.updateInfo.compositesToUpdateReplacements.size > 0) {
await this.replacementChangesFromCompositesToUpdate();

let componentNamesToUpdate = [...this.updateInfo.componentsToUpdateRenderers];
this.updateInfo.componentsToUpdateRenderers.clear();
await this.processStateVariableTriggers();

await this.updateRendererInstructions({
componentNamesToUpdate,
sourceOfUpdate: { sourceInformation, local: true },
actionId,
});
if (!skipRendererUpdate) {
await this.updateAllChangedRenderers(sourceInformation, actionId);
}

await this.processStateVariableTriggers();

// TODO: when should we actually warn of unmatchedChildren
// It shouldn't be just on update, but also on initial construction!
// Also, should be more than a console.warn
Expand Down Expand Up @@ -8601,6 +8594,37 @@ export default class Core {

}

async updateAllChangedRenderers(sourceInformation = {}, actionId) {

let componentNamesToUpdate = [...this.updateInfo.componentsToUpdateRenderers];
this.updateInfo.componentsToUpdateRenderers.clear();

await this.updateRendererInstructions({
componentNamesToUpdate,
sourceOfUpdate: { sourceInformation, local: true },
actionId,
});


// updating renderer instructions could trigger more composite updates
// (presumably from deriving child results)
// if so, make replacement changes and update renderer instructions again
// TODO: should we check for child results earlier so we don't have to check them
// when updating renderer instructions?
if (this.updateInfo.compositesToUpdateReplacements.size > 0) {
await this.replacementChangesFromCompositesToUpdate();

let componentNamesToUpdate = [...this.updateInfo.componentsToUpdateRenderers];
this.updateInfo.componentsToUpdateRenderers.clear();

await this.updateRendererInstructions({
componentNamesToUpdate,
sourceOfUpdate: { sourceInformation, local: true },
actionId,
});
}
}

requestRecordEvent(event) {

this.resumeVisibilityMeasuring();
Expand Down Expand Up @@ -9173,7 +9197,7 @@ export default class Core {
inverseDefinitionArgs.stateValues = component.stateValues;
inverseDefinitionArgs.overrideFixed = instruction.overrideFixed;
inverseDefinitionArgs.shadowedVariable = instruction.shadowedVariable;
inverseDefinitionArgs.sourceInformation = instruction.sourceInformation;
inverseDefinitionArgs.sourceDetails = instruction.sourceDetails;


let stateVariableForWorkspace = stateVariable;
Expand Down
Loading