There's no point in first checking if a value exists in a Set before deleting it, and this is just a left-over from before the code used Maps and Sets.
Also, remove a couple of unnecessary comments describing how `Set.prototype.add` works (when those where added there wasn't much Set usage in the code-base).
In cases where all the requested chunks are already available, and dispatching a range-request thus isn't necessary, there's no point in creating an unused `requestId` nor keeping track of it since it'll never be accessed given the early return.
The `FontInfo` class doesn't use the `extra` properties for anything, and they are already exposed via the `FontFaceObject` instance which is how the debuggers access those properties.
Prior to PR 20197 each font-instance had just a single copy of all its relevant font-properties on the main-thread, however that's unfortunately no longer the case.
Given how the compilation was implemented, every single time that a font-property is accessed on the main-thread it'll now be re-parsed. Not only does this seem inefficient, especially for properties needed e.g. during text-rendering, but it'll lead to (potentially) a lot of duplicated object creation.
Consider what currently happens when rendering all pages of the following PDFs:
- `tracemonkey.pdf` contains `24` separate fonts, but the `fontMatrix`-property is re-parsed a whopping `1009` times (once per `showText` operator).
- `standard_fonts.pdf` contains `14` separate fonts, however we create no less than `186` separate `StandardFontInfo` instances.
- `xfa_bug1716816.pdf` contains `4` separate fonts, however we create no less than `45` separate `CssFontInfo` instances.
This is obviously not limited to just the font-properties listed above, but those are mere examples to illustrate the problem.
By shadowing the font-property getters on the main-thread, obviously with the exception of `FontInfo.prototype.data`, we only need to parse each font-property *once* per font-instance.
*Note:* Unfortunately this *increases* the size of the `gulp mozcentral` bundle by `792` bytes, but that cannot really be helped since this seems like the correct thing to do regardless.
Currently the `FontInfo.prototype.{data, cssFontInfo, systemFontInfo}` getters duplicate virtually the same code when reading buffer-data, which seems completely unnecessary.
*Note:* This reduces the size of the `gulp mozcentral` bundle by `738` bytes, and with the upcoming worker-rendering this saving will be doubled.
- All embedded font data, regardless of how it's specified in the PDF, is always converted into OpenType in the worker-thread. This has been the case since "forever" in the PDF.js project, hence the value of `Font.prototype.mimetype` never varies (when actually set).
- With the introduction of the CSS Font Loading API, in the font-loading code, the `mimetype` property is no longer used *by default* in the main-thread.
- Given that `Font.prototype.mimetype` is either a string or `null`, the way that PR 20197 implemented the serialization/deserialization isn't actually correct since an explicit `null` value is being converted into a `"null"` string.
After PR 20933 the `Glyph.prototype.vmetric` property is now guaranteed to be always be defined for vertical fonts, since it'll fallback to the `defaultVMetrics` property; see https://github.com/mozilla/pdf.js/blob/18e8a26a3813a319b38c806076f0b0ef9baf1bf4/src/core/fonts.js#L3564
Hence it's no longer necessary to export the `defaultVMetrics` as part of the compiled Font info-data.
Additionally, with `Glyph.prototype.vmetric` always being defined, there's fallback code in both the worker/main-thread that should no longer be necessary.
The `guessFallback` parameter is used during system-font parsing on the worker-thread, to ensure that the fallback-name is only appended once.
However, with the exception of the unit-tests this is completely unused on the main-thread.
This being a single byte of data isn't going to make any noticeable difference, however it does allow us to remove "unnecessary" code.
After the previous patches the total length of the encoded strings are consistently available *before* writing them, hence we can directly set the length and remove the temporary (i.e. zero) placeholder entry.
Currently `compileCssFontInfo`, `compileSystemFontInfo`, and `compileFontInfo` duplicate the same exact code for encoding and writing strings, which can be avoided with the introduction of two new helpers.
*Note:* This reduces the size of the `gulp mozcentral` bundle by `450` bytes, which isn't a lot but still cannot hurt.
Currently we duplicate the same exact code *twice* when writing the `systemFontInfoBuffer` and `cssFontInfoBuffer` data.
*Note:* This reduces the size of the `gulp mozcentral` bundle by `271` bytes, which isn't a lot but still cannot hurt.
Currently the `CssFontInfo`, `SystemFontInfo`, and `FontInfo` classes duplicate virtually the same code when reading string-data, which seems completely unnecessary.
The `SystemFontInfo.prototype.style` getter can also be re-factored to use the new helper function, rather than manually reading those strings.
*Note:* This reduces the size of the `gulp mozcentral` bundle by `627` bytes, and with the upcoming worker-rendering this saving will be doubled.
These functions no longer need to be `async` after PR 19026, since no value is being returned now, and instead we can directly return the `StructTreeRoot.createStructureTree` respectively `StructTreeRoot.prototype.updateStructureTree` call since both methods (implicitly) return undefined.
The de-serialization of Axial shading-patterns, implemented in PR 20340, will currently lead to the creation of unnecessary intermediate `Float32Array`s. Instead we can create the final Arrays *directly*, the same way as is done for Radial shading-patterns.
While this is an improvement the data is small and simple enough that it's not going to be measurable, however as a matter of principle we should nonetheless always avoid pointless copying of data.
The CMap-data and the ToUnicode-data is often sparse[1], which means that Arrays are not ideal data-structures for this purpose.
Note how there are separate paths, in the existing code, depending on the size of the data and that we're forced to either iterate over non-existing keys or use `for...in` iteration which "unnecessarily" stringify the keys.
By using Maps instead both of these issues can be avoided, and given that performance of Maps have been improved recently (in Firefox) this shouldn't be an issue.
Given how intertwined all of this functionality is, it unfortunately wasn't really possible to easily split this into several patches.
However, all of this code should (famous last words) be well covered by existing test-cases.
One notable difference is that iterating through CMap-data and ToUnicode-data now happens in insertion order, but given how this data is being used that's likely not an issue.
Also, copy the data returned by `CMap.prototype.getMap` since it's used as input to the `ToUnicodeMap` class. Note that we may amend the ToUnicode-data at the end of font parsing, hence we should not modify the underlying CMap-data.
Given how/where the CMap-data is accessed it's unlikely that this pre-existing "bug" has caused any issues, but it nonetheless seems like something that should be fixed.
*Note:* We purposely keep the `forEach` methods, since making the classes iterable seemed to be approximately an order or magnitude slower (based on very quick `console.{time, timeEnd}` benchmarking).
---
[1] In some cases even *extremely* sparse, see e.g. `issue8372.pdf`.
Parts of the Worker loading code is really old (well over a decade), and goes back to a time where not all browsers supported Workers or had incomplete/broken implementations.
At this point in time it thus seems reasonable to assume that if Workers are available they first of all work correctly, and secondly that they support `postMessage` transfers.
Hence this patch, which suggests that we (ever so slightly) speed up the Worker loading by not having to wait for a separate "test" message.
Instead the worker-thread will send a test-object with the "ready" message, and we'll check on the main-thread that the expected data was received.
For the GENERIC viewer, on a fast laptop, this patch reduces Worker load-times between 1 and 6 milliseconds (the measurements are somewhat noisy).
While this obviously isn't a lot it cannot hurt, and it may be more significant on slower hardware.
*Note:* This won't improve loading performance of the Firefox PDF Viewer, since it uses a pre-loaded Worker nowadays.
However, the upcoming renderer-worker looks to be modelled on the existing `PDFWorker` and if we're going to be creating more Workers having their loading be as fast as possible seems like a worthy goal.
In practice `toFontChar` is always a more or less sparse Array, and sometimes it's even *extremely* sparse (see e.g. `issue8372.pdf`), which means that a Map seems like a more appropriate data-structure.
In practice the Differences-data is a more or less sparse Array[1], which means that a Map seems like a more appropriate data-structure.
---
[1] Even in the `tracemonkey.pdf` document it's at most 254 entries, out of the maximum 256 ones, but with most of the fonts having considerably fewer entries.
This is a follow-up to *the second* commit in PR 20861, since we can also avoid a little bit of effectively duplicated code when writing the `bbox`, `fontMatrix`, and `defaultVMetrics` data.
*Note:* This reduces the size of the `gulp mozcentral` bundle by `239` bytes, which isn't a lot but still cannot hurt.
Given that the full PDF.js library won't even run if `Object.prototype` has been incorrectly extended, it seems redundant to generate just these two expected-value Objects with an explicit `null` prototype.
Besides, we already compare against "regular" Objects all over the unit-tests without doing anything similar.
Rather than re-creating this function for every single "simple" operation, in `PartialEvaluator.prototype.getOperatorList`, we can define it just once instead.
Even for relatively simple PDFs, e.g. `tracemonkey.pdf`, this avoids creating a few thousand copies of that function when rendering all pages. For much larger PDFs, e.g. `pdf.pdf`, this avoids a few hundred thousand copies of that function.
After the previous commit throwing an Error now results is (slightly) more code than keeping the `#testFontLoaded` call intact, and note that that method itself already throws in MOZCENTRAL builds.
Also, fix a typo in an error message thrown in WORKER_THREAD builds.
The existing methods are only used as a fallback in non-Firefox browsers that lack support for the Font Loading API, and this is also very old code.
By combining this functionality into just a single (private) method, we ever so slight reduce the size of this code in the MOZCENTRAL build.
This method is only used as a fallback in non-Firefox browsers that lack support for the Font Loading API, hence by inlining that data we reduce the bundle size of the MOZCENTRAL build a tiny bit.
After PR 21938 the integration-tests have started failing intermittently, but only in Google Chrome, so let's wait for *all* fields explicitly rather than just one of them.
Currently we localize all of the necessary strings "manually", which is a pattern that we've moved away from in the code-base.
Instead we'll now set the "data-l10n-id" and "data-l10n-args" attributes on the relevant DOM elements, and let Fluent handle the localization automatically, which helps reduce overall asynchronicity in the `PDFDocumentProperties` code.
*Note:* We purposely keep the `#updateUI` helper, rather than updating l10n-attributes piecemeal, since it ensures that the dialog always displays consistent state.
Compared to all the other unit-tests in the "PDFWorker" describe-block this one doesn't actually depend on Workers being available, since it only checks basic API functionality.
The reason that this unit-test was disabled in Node.js is that prior to PR 17055 the `GlobalWorkerOptions.workerSrc` option wasn't guaranteed to be set there.
All of this code directly, or indirectly, assumes that the DOM is available which (obviously) isn't the case in workers.
This will help reduce the bundle size impact of the upcoming worker-rendering, by stubbing out code that cannot run there.
Currently this code is accessed with a dynamic import, which has the unfortunate side-effect of inflating the size of the *built* `pdf.worker.mjs` file a whole lot.
Given the relatively small size of the `src/core/editor/print_appearances.js` file it really doesn't seem like an issue to just bundle that code unconditionally, and the existing pre-processor checks means that the `importPrintedAppearances` code will only be invoked in MOZCENTRAL builds.
*NOTE:* This patch reduces the size of the `gulp mozcentral` bundle by `96` kilo-bytes, which I think is way too much to ignore.
The `ViewHistory.prototype.set` method is first of all not used a lot, and secondly it's not used in any hot code-paths[1].
Hence we can use `setMultiple` consistently instead, which means that the `ViewHistory.prototype.set` method can be removed.
---
[1] Only when opening/closing the sidebar or changing sidebar view, and when changing scroll/spread modes in the viewer.
This seems like a more appropriate data-structure, rather than storing `true` values in a regular Object.
Also, fix existing inconsistencies in the `CMap.prototype.forEach` and `ToUnicodeMap.prototype.forEach` methods since they didn't always return integer charCodes. Note how various `forEach` call-sites previously did that manually, which seems like a "wrong" solution.
After PR 21891 the `interpolate` function is basically duplicated, since the stitched-handling currently inline effectively identical code.
Also, fix existing typos in a couple of method/function names ("Stiched" -> "Stitched").
In hindsight the *second* commit of PR 21888, which was intended to improve how `Ref.fromString` handles "bad" arguments, seems wrong since that method now accepts arguments that don't agree fully with the string representation used in the `Ref.get` method.
The `Ref.fromString` method will now properly validate the argument, and "reject" (i.e. return `null`) unless it matches the expected format.
If the reference-string argument is malformed, see the updated unit-tests, we could miss existing cache entries and thus incorrectly re-create `Ref` instances.
Note that this *should* never happen in practice, but it nonetheless ought to be fixed.
Rather than splitting the filename into an Array, and then take its last element, we can directly search for the *last* dot instead.
Doing this also fixes what appears to be a small oversight, since the current code may find a "valid" extension when one doesn't actually exist in the filename. For example, try running the following code in the console:
```js
var filename = "mp4";
filename.split(".").at(-1)?.toLowerCase();
```
This changes `seacMap` from a regular Object into a Map, and the Type1/CFF `seacs` from (potentially very) sparse Arrays into Maps.
*Note:* These changes are covered by existing ref-tests such as: `issue818.pdf`, `issue4801`, `issue4573.pdf`, `bug1308536.pdf`, and `glyph_accent.pdf`.
This parameter hasn't been used anywhere in the code-base after the introduction of Fluent, hence it can simply be removed now.
Furthermore, note also that the `L10n.prototype.get` method itself is not used very much nowadays.
Given that Fluent supports localizing multiple strings "at once", we can replace the current two or three calls (depending on page size) with just a single one.
This also, indirectly, improves test coverage of the `L10n` class since the "multiple ids branch" previously wasn't used anywhere in the code-base.
Also remove an "ancient" comment, from the "GetOperatorList" handler, which no longer seems helpful.
Finally, since they're now unused, remove the return-values from the `Page.prototype.getOperatorList` method.
The only actual `getOutputLength` call-site, outside of the unit-tests, was removed in PR 21637 and these methods are now unused.
This code can always be easily re-instated if needed, thanks to version control, so let's avoid shipping dead code in the builds.
Jonas Jenwald