Closed
Bug 1396026
Opened 7 years ago
Closed 7 years ago
Graphite automatic collision fixing no longer works.
Categories
(Core :: Graphics: Text, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | unaffected |
firefox57 | --- | fixed |
People
(Reporter: mark, Assigned: kevin.hsieh)
References
()
Details
(Keywords: regression, Whiteboard: [gfx-noted])
Attachments
(1 file)
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.
Comment 1•7 years ago
|
||
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.
Assignee | ||
Comment 2•7 years ago
|
||
I'll look into this.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → kevin.hsieh
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•7 years ago
|
||
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 hidden (mozreview-request) |
Comment 5•7 years ago
|
||
mozreview-review |
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+
Updated•7 years ago
|
Attachment #8905613 -
Flags: review?(nfroyd)
Comment 7•7 years ago
|
||
mozreview-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)
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Priority: -- → P3
Whiteboard: [gfx-noted]
Comment 9•7 years ago
|
||
mozreview-review |
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+
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 11•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/35dcbab0bdbd Update OTS to accept Awami Nastaliq. r=froydnj,jfkthame
Keywords: checkin-needed
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/35dcbab0bdbd
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Updated•7 years ago
|
Blocks: 1369672
status-firefox55:
--- → unaffected
status-firefox56:
--- → unaffected
status-firefox-esr52:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•