Closed Bug 1396026 Opened 2 years ago Closed 2 years ago
Graphite automatic collision fixing no longer works
59 bytes, text/x-review-board-request
Graphite issue: Awami Nastaliq automatic collision fixing no longer works in Nightly. This worked properly before (ESR52, Fx 55). See the URL for a demo.
Looks like the OTS Graphite support is rejecting this font: it shows console messages downloadable font: Glat: Decompression failed (font-family: "AwamiT" style:normal weight:normal stretch:normal src index:0) source: http://scripts.sil.org/pub/woff/fonts/AwamiNastaliq-Regular.woff unknown:80:11 downloadable font: Dropping all Graphite tables (font-family: "AwamiT" style:normal weight:normal stretch:normal src index:0) source: http://scripts.sil.org/pub/woff/fonts/AwamiNastaliq-Regular.woff So we need to look into that, and figure out why it's failing.
I'll look into this.
The Awami Nastaliq font has padding after its compressed data, so LZ4 complains about unconsumed input. I've submitted an upstream PR that accounts for this. The font also has fields that are calculated in a way that is inconsistent with older Graphite fonts; jfkthame has submitted relevant upstream PR's.
Comment on attachment 8905613 [details] Bug 1396026 - Update OTS to accept Awami Nastaliq. https://reviewboard.mozilla.org/r/177410/#review182428 r=me for the OTS update here, but I think we should also ask one of the mfbt owners about the API change in Compression.h ... are they OK with adding the optional `partial` parameter, or would an alternative approach be preferred? (Another option would be to add a separate LZ4::decompressPartial wrapper to expose the LZ4 function we need.)
Attachment #8905613 - Flags: review?(jfkthame) → review+
Comment on attachment 8905613 [details] Bug 1396026 - Update OTS to accept Awami Nastaliq. https://reviewboard.mozilla.org/r/177410/#review182526 ::: mfbt/Compression.h:93 (Diff revision 1) > + * @param partial is set to true when performing a partial decompression > + * (i.e., to ignore unconsumed input upon reaching aMaxOutputSize) > * @return true on success, false on failure > */ > static MFBT_API MOZ_MUST_USE bool > decompress(const char* aSource, size_t aInputSize, char* aDest, > - size_t aMaxOutputSize, size_t* aOutputSize); > + size_t aMaxOutputSize, size_t* aOutputSize, bool partial = false); Drive-by stylistic nit: s/partial/aPartial/ (In the function signature & documentation in the .h file, and in the impl in the .cpp file)
2 years ago
Priority: -- → P3
Comment on attachment 8905613 [details] Bug 1396026 - Update OTS to accept Awami Nastaliq. https://reviewboard.mozilla.org/r/177410/#review183800 Sorry for the delay in looking at this. r=me with the changes below; please remember to add a documentation comment for the new function. Thanks! ::: mfbt/Compression.cpp:72 (Diff revision 2) > - int ret = LZ4_decompress_safe(aSource, aDest, inputSizeChecked.value(), > + int ret = aPartial ? LZ4_decompress_safe_partial(aSource, aDest, > + inputSizeChecked.value(), > + maxOutputSizeChecked.value(), > + maxOutputSizeChecked.value()) > + : LZ4_decompress_safe(aSource, aDest, > + inputSizeChecked.value(), > - maxOutputSizeChecked.value()); > + maxOutputSizeChecked.value()); I think I have a small preference for exposing this as a separate function, `LZ4::decompressPartial`, for consistency with the exposed LZ4 API.
Attachment #8905613 - Flags: review?(nfroyd) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/35dcbab0bdbd Update OTS to accept Awami Nastaliq. r=froydnj,jfkthame
You need to log in before you can comment on or make changes to this bug.