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

Kb 48270 #990

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Kb 48270 #990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d84b601
feat(viewer): viewer performance improvement
xjiang-at-wiris Jun 19, 2024
198dab2
fix: text bug fixes
xjiang-at-wiris Jul 24, 2024
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
15 changes: 15 additions & 0 deletions demos/html/viewer/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ <h4>Commit hash: <output id="git_commit"></output></h4>
value="https://www.wiris.net/demo/plugins/app/"
/>
</label>
<label>
Simultaneous MathML petition
<input name="simultaneousmml" id="simultaneousmml" type="number" value="50" />
</label>
<label>
Viewer Offset
<input name="vieweroffset" id="vieweroffset" type="number" value="200" />
</label>
<div class="checkbox-container">
<label for="dpi">DPI:</label>
<input name="dpi" id="dpi" type="range" min="0" max="100" value="96" />
Expand All @@ -62,6 +70,13 @@ <h4>Commit hash: <output id="git_commit"></output></h4>
Language:
<input name="lang" id="lang" type="text" value="en" />
</label>
<label>
Simultaneous LaTeX petition
<input name="simultaneouslatex" id="simultaneouslatex" type="number" value="50" />
</label>
<label>
<!-- PLACEHOLDER -->
</label>
<div class="checkbox-container">
<label for="zoom">Zoom:</label>
<input name="zoom" id="zoom" type="range" min="0" max="10" step="0.01" value="1" />
Expand Down
4 changes: 3 additions & 1 deletion demos/html/viewer/src/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ document.getElementById("viewer_form").addEventListener("submit", () => {
lang: lang.value,
viewer: viewer.value,
zoom: zoom.value,
vieweroffset:vieweroffset.value,
simultaneousmml:simultaneousmml.value,
simultaneouslatex:simultaneouslatex.value,
};

window.viewer.properties.config = config;

document.getElementsByTagName("main")[0].innerHTML = document.getElementById("content").value;
window.com.wiris.js.JsPluginViewer.parseElement(document.getElementsByTagName("main")[0]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has been removed because we no longer need "Render under request."

});
1 change: 1 addition & 0 deletions demos/html/viewer/src/static/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ body {
}

input[type="text"],
input[type="number"],
select,
textarea {
width: 100%;
Expand Down
3 changes: 3 additions & 0 deletions packages/viewer/docs/spec.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ The following table contains a specification of each of the properties.
| lang | The language for the alt text. | Frontend | | en |
| dpi | Resolution in dots per inch of the generated image. This feature scales the formula with a factor of dpi/96. | Frontend | Positive integer | 96 |
| zoom | The scale of the generated image. | Frontend | Positive floating point number | 1 |
| vieweroffset | We render only the formulas on screen. This parameter defines the number of pixels to render in advance. | Frontend | Positive floating point number | 200 |
| simultaneousmml | Max number of possible simultaneous MathML rendering petitions at the same time. | Frontend | Positive floating point number | 50 |
| simultaneouslatex | Max number of possible simultaneous LaTeX rendering petitions at the same time. | Frontend | Positive floating point number | 50 |

## API

Expand Down
106 changes: 71 additions & 35 deletions packages/viewer/src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Properties } from "./properties";
import { renderLatex } from "./latex";
import { renderMathML } from "./mathml";
import { renderAMathML } from "./mathml";
import { bypassEncapsulation } from "./retro";
import packageInformation from "../../../node_modules/@wiris/mathtype-viewer/package.json";

Expand Down Expand Up @@ -41,49 +41,85 @@ async function main(w: Window): Promise<void> {

const document = w.document;

/**
* Parse the DOM looking for LaTeX and <math> elements.
* Replaces them with the corresponding rendered images within the given element.
* @param {Document} document - The DOM document in which to search for the rendering root.
* @param {MutationObserver} observer - Mutation observer to activate or reactivate every time the rendering root changes.
*/
properties.render = async () => {
const element: HTMLElement = document.querySelector(properties.element);
if (element) {
await renderLatex(properties, element);
await renderMathML(properties, element);
}
};

// Initial function to call once document is loaded
// Renders formulas and sets observer
const start = async () => {
// Check if the viewer is alredy loaded
// Check if the viewer is already loaded
if (w.viewer.isLoaded) return;

w.viewer.isLoaded = true;

// First render
properties.render();
// Interaction Observer
const mathobserver = new IntersectionObserver(async (entries, observer) => {

let latexPromises = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that you are using the simultaneousmml but I fail to find the place where the simultaneouslatex is being used. It seems then that the parameter is not providing any value to the viewer at this point. Is this intentional, or am I failing to see something?

Copy link
Contributor

@psala-at-wiris psala-at-wiris Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Same here!
In Latex.ts, line 21:

 const maxLatexPetition = properties.simultaneousmml;

Maybe maxLatexPetition should be equal to properties.simultaneouslatex?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My mistake! It should be const maxLatexPetition = properties.simultaneouslatex;
FIxed, thank you both!


const maxMmlPetition = properties.simultaneousmml;
let mmlPromises = [];
for (const entry of entries) {

if (entry.isIntersecting && entry.target instanceof MathMLElement && entry.target.matches('math')) {
const promise = renderAMathML(properties, entry.target);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I am missing something, but:
Why the renderAMathMl method does not implement the petition guard and renderLatex does?
Could be possible to implement the guard logic in the same way for both methods? For me is kind of confusing that one does the guard mechanism and the other doesn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we talked in private, renderAMathMl and renderLatex have some different behavior, that's why they don't share the same guard mechanism

mmlPromises.push(promise);
observer.unobserve(entry.target);

// To not saturate service, not make all render petition at once
if (mmlPromises.length >= maxMmlPetition) {
await Promise.all(mmlPromises);
mmlPromises = [];
}
}

if (entry.isIntersecting && entry.target instanceof HTMLElement) {
const promise = renderLatex(properties, entry.target);
latexPromises.push(promise);
observer.unobserve(entry.target);
}
}
await Promise.all(latexPromises);
await Promise.all(mmlPromises);
}, {
rootMargin: `${properties.vieweroffset}px`
});

const allElements = document.querySelectorAll('*');
window.addEventListener("load", function () {
allElements.forEach(function (element) {
mathobserver.observe(element);
});
});

//MutationObserver
// Callback called every time there is a mutation in the watched DOM element
// Feature temporarily disabled due to KB-37834
// new MutationObserver(async (mutationList, observer) => {
// for (const mutation of mutationList) {
// for (const node of mutation.addedNodes) {
// if (node instanceof HTMLElement) {
// await properties.render();
// }
// }
// }
// })
// // We need to watch over the whole document, in case the Properties.element is inserted
// // e.g. we set Properties.element = '#renderArea' and then we append <div id="renderArea">$$2+2=4$$</div> to the document
// .observe(document, {
// attributes: true, // In case an attribute is changed in a <math> node, for instance
// childList: true, // In case a new <math> or $$latex$$ node is added, for instance
// subtree: true, // In case a <math> node is added as a descendant of the observed element, for instance
// });
new MutationObserver(async (mutationList) => {
for (const mutation of mutationList) {
for (const node of mutation.addedNodes) {
// If was a htmlelement, decompose to child element to find posible latex or mathml
if (node instanceof HTMLElement) {
const allElements = node.querySelectorAll('*');

allElements.forEach(function (element) {
mathobserver.observe(element);
});
}
// Observe mathml
else if (node instanceof MathMLElement) {
mathobserver.observe(node);
}
// If it was a pure text, observe their parent in order to find posible latex
else if (node.nodeType == Node.TEXT_NODE) {
mathobserver.observe(node.parentElement);
}
}
}
})
// We need to watch over the whole document, in case the Properties.element is inserted
// e.g. we set Properties.element = '#renderArea' and then we append <div id="renderArea">$$2+2=4$$</div> to the document
.observe(document, {
attributes: true, // In case an attribute is changed in a <math> node, for instance
childList: true, // In case a new <math> or $$latex$$ node is added, for instance
subtree: true, // In case a <math> node is added as a descendant of the observed element, for instance
});
};

// https://developer.mozilla.org/en-US/docs/Web/API/Document/DOMContentLoaded_event#checking_whether_loading_is_already_complete
Expand Down
14 changes: 13 additions & 1 deletion packages/viewer/src/latex.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,21 @@ export async function renderLatex(properties: Properties, root: HTMLElement) {

const latexNodes = findLatexTextNodes(root);

const maxLatexPetition = properties.simultaneouslatex;

let promises = [];

for (const latexNode of latexNodes) {
await replaceLatexInTextNode(properties, latexNode);
promises.push(replaceLatexInTextNode(properties, latexNode));

// To not saturate service, not make all render petition at once
if (promises.length >= maxLatexPetition) {
await Promise.all(promises);
promises = [];
}
}

await Promise.all(promises);
}

/**
Expand Down
94 changes: 68 additions & 26 deletions packages/viewer/src/mathml.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,41 @@ function decodeSafeMathML(root: HTMLElement) {
}
}

export async function renderAMathML(properties: Properties, mathElement: MathMLElement): Promise<void> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to delete the renderMathML function, but it seems to have a lot of implications and requires many document modifications. However, it appears worth discussing its feasibility.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would like so too. I would even consider this part of the development of the task, since the renderMathML is used for the retro-compatibility methods, which could benefit from the new performance improvements.

It would require a part of investigation and maybe adapt the current methods so that renderMathML can be properly replaced.

if (properties.viewer !== "image" && properties.viewer !== "mathml") {
return;
}

const mml = serializeHtmlToXml(mathElement.outerHTML);

try {
let imgSource;

// Transform mml to img.
if (properties.wirispluginperformance === "true") {
imgSource = await showImage(
mml,
properties.lang,
properties.editorServicesRoot,
properties.editorServicesExtension,
);
} else {
imgSource = await createImage(
mml,
properties.lang,
properties.editorServicesRoot,
properties.editorServicesExtension,
);
}

// Set img properties.
const img = await setImageProperties(properties, imgSource, mml);
// Replace the MathML for the generated formula image.
mathElement.parentNode?.replaceChild(img, mathElement);
} catch {
console.error(`Cannot render ${mml}: invalid MathML format.`);
}
}
/**
* Parse the DOM looking for <math> elements and replace them with the corresponding rendered images within the given element.
* @param {Properties} properties - Properties of the viewer.
Expand All @@ -60,37 +95,44 @@ export async function renderMathML(properties: Properties, root: HTMLElement): P

decodeSafeMathML(root);

const promises = [];

for (const mathElement of [...root.getElementsByTagName("math")]) {
const mml = serializeHtmlToXml(mathElement.outerHTML);
try {
let imgSource;

// Transform mml to img.
if (properties.wirispluginperformance === "true") {
imgSource = await showImage(
mml,
properties.lang,
properties.editorServicesRoot,
properties.editorServicesExtension,
);
} else {
imgSource = await createImage(
mml,
properties.lang,
properties.editorServicesRoot,
properties.editorServicesExtension,
);
const promise = (async () => {
try {
let imgSource;

// Transform mml to img.
if (properties.wirispluginperformance === "true") {
imgSource = await showImage(
mml,
properties.lang,
properties.editorServicesRoot,
properties.editorServicesExtension,
);
} else {
imgSource = await createImage(
mml,
properties.lang,
properties.editorServicesRoot,
properties.editorServicesExtension,
);
}

// Set img properties.
const img = await setImageProperties(properties, imgSource, mml);
// Replace the MathML for the generated formula image.
mathElement.parentNode?.replaceChild(img, mathElement);
} catch {
console.error(`Cannot render ${mml}: invalid MathML format.`);
}
})();

// Set img properties.
const img = await setImageProperties(properties, imgSource, mml);
// Replace the MathML for the generated formula image.
mathElement.parentNode?.replaceChild(img, mathElement);
} catch {
console.error(`Cannot render ${mml}: invalid MathML format.`);
continue;
}
promises.push(promise);
}

await Promise.all(promises);
}

/**
Expand Down
Loading
Loading