[graf2d] Make libjpeg a builtin obtained from the web and remove the vendored code#21556
Conversation
this change simply add the new code, while the previous mechanism, i.e. the vendoring of libjpeg, is still used.
There was a problem hiding this comment.
Awesome work, thanks!
In the future, is there a plan to follow the same path with libpng ?
One unrelated question: I see there are two different zlib builtin versions hard-coded in the repo, once in graf2d/asimage/src/libAfterImage/zlib and once in builtins/zlib/
Is that a historical artefact or is it on purpose?
|
Hi @ferdymercury ! Yes of course! And more simplifications are planned, and I suspect we'll be able to provide even more as we continue to work on this problem |
hageboeck
left a comment
hageboeck
left a comment
There was a problem hiding this comment.
It's great to see this coming up, but I have a few questions. 🙂
| for (auto &&fileName : fileNames) { | ||
| c.SaveAs(fileName.c_str()); | ||
| } |
There was a problem hiding this comment.
How does it test that the images are written correctly? Is ROOT issuing a TError, so a problem would be detected?
There was a problem hiding this comment.
If the answer is no, one could check that the files size exceeds a value to be determined by trial and error.
I also would prefer having a canvas that's not completely empty. Could we e.g. put a title box or a histogram on it?
There was a problem hiding this comment.
the answer to the first question is yes. And yes, we could have a title and a histo, no doubt about that.
thank you for the guidance! |
Test Results 23 files 23 suites 3d 6h 33m 33s ⏱️ For more details on these failures, see this check. Results for commit ea5d185. ♻️ This comment has been updated with latest results. |
dpiparo
force-pushed
the
builtin_libjpeg_v2
branch
from
263956a to
13b3c7e
Compare
|
All comments addressed and also corrected a typo in the name of the prefix directory (that would not have had any impact on users, but still) |
dpiparo
force-pushed
the
builtin_libjpeg_v2
branch
from
13b3c7e to
65c2670
Compare
gifs are not tested yet because of root-project#21561
dpiparo
force-pushed
the
builtin_libjpeg_v2
branch
from
65c2670 to
ea5d185
Compare
|
the gif generation has been put aside because of #21561 . |
4 participants
This PR accomplishes these steps:
Thanks to @hageboeck and @bellenot for all their help in crafting this changes!