Summary: | REGRESSION(275829@main): [GTK][WPE] Images broken in various PDFs | ||
---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> |
Component: | Assignee: | Michael Catanzaro <mcatanzaro> | |
Status: | NEW --- | ||
Severity: | Normal | CC: | a_protyasha, bugs-noreply, cgarcia, mcatanzaro, sabouhallawa |
Priority: | P2 | ||
Version: | Other | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=273435 https://bugs.webkit.org/show_bug.cgi?id=271726 |
Description
Michael Catanzaro
2024-08-26 13:27:20 PDT
Plan: * Revert 273978@main, bring back OpenJPEG for now * Future improvement: ideally try to limit JPEG 2000 to PDF documents only rather than allowing it on the entire web * Future improvement: try building OpenJPEG for wasm as part of the PDF.js build process, bug #273435, so we can remove system OpenJPEG dependency again (In reply to Michael Catanzaro from comment #1) > * Revert 273978@main, bring back OpenJPEG for now Bad plan. After attempting this, I spent a while trying to figure out why images in PDF documents were still broken. Turns out, this commit actually didn't break anything. The only other suspicious commit is 277629@main, but that also didn't break things. I will need to bisect this. Wow, we actually have multiple bugs. Test document one is https://bug-271726-attachments.webkit.org/attachment.cgi?id=470613 Test document two is https://insurance.mo.gov/earthquake/documents/EarthquakeInsuranceMarketsInMissouriReport20197-8-2019_000.pdf They did not break at the same time. At the first step of my bisect, I can display test document one, but not test document two. :S So now I'll bisect twice. (In reply to Michael Catanzaro from comment #3) > Test document two is > https://insurance.mo.gov/earthquake/documents/ > EarthquakeInsuranceMarketsInMissouriReport20197-8-2019_000.pdf Unfortunately this broke in 275829@main "[GTK] Make it possible to build with Skia instead of Cairo". Let's focus this bug report on this issue. I will create a separate bug report for the problem with test document one. (In reply to Michael Catanzaro from comment #4) > Unfortunately this broke in 275829@main "[GTK] Make it possible to build > with Skia instead of Cairo". Building with -DUSE_CAIRO=ON does *not* solve the problem, which I suppose is unsurprising, since that commit does not yet actually switch the default from USE_CAIRO to USE_SKIA. I have no guesses as to what aspect of that change breaks PDF images. (In reply to Michael Catanzaro from comment #4) > Let's focus this bug report on this issue. I will create a separate bug > report for the problem with test document one. Test document one displays fine in my development build. It is only broken in Ephy Tech Preview. So I cannot bisect it. (In reply to Michael Catanzaro from comment #5) > Building with -DUSE_CAIRO=ON does *not* solve the problem, which I suppose > is unsurprising, since that commit does not yet actually switch the default > from USE_CAIRO to USE_SKIA. I have no guesses as to what aspect of that > change breaks PDF images. Well today it most certainly does. I made a stupid mistake: the default value of CMake variables doesn't change unless you delete your build directory, and I didn't do that when bisecting. So that's good news actually. To avoid the regression, we can simply switch back to USE_CAIRO=ON and USE_SKIA=OFF. That would be a quick/easy solution for 2.46. Even better news is that https://github.com/WebKit/WebKit/pull/33122 fixes the issue Nice, thanks for investigating! |