Add docker support, fix smaller dependency issues in javascript and fix outdated "flattenImage" method #11
Conversation
| @@ -0,0 +1,38 @@ | |||
| <?xml version="1.0"?> | |||
There was a problem hiding this comment.
How's this file loaded and used?
There was a problem hiding this comment.
It took me quite a while to get it to run. This file is copied into the docker container. The command
RUN fc-cache -f -v`
in Dockerfile then processes it. All linux processes working with fonts then use these hints to do the font rendering, including PHP/Imagemagick.
Here is some more information about fc-cache/fontconfig: https://linux.die.net/man/1/fc-cache
And the result of the file is that the fonts are rendered without antialiasing which results in more crisp text. In a display with gray scales antialiasing is great but on a pure black & white display it is distorting the text
There was a problem hiding this comment.
Ah so it's a docker specific thing. Gotcha!
| // Image rendering stuff | ||
|
|
||
| function renderSVG($id) { | ||
| header('Content-type: image/svg+xml'); |
There was a problem hiding this comment.
Why removing the content type? I guess it might still work but I also don't see the reason for it. Is this wrong in any way?
There was a problem hiding this comment.
Good point, I actually planned to investigate a bit more why this causes a problem. My current assumption is: rendering the bitmap formats also calls this function. Setting the header here writes the header 'Content-type: image/svg+xml' into the png/proprietary bitmap output stream. If I don't remove this line the bitmap is not displayed on the home screen of the admin page. Probably the header(..) call should be moved to the place where svg, png and proprietary bitmap are processed.
There was a problem hiding this comment.
Errr good catch! Yeah this should indeed be moved in that case.
However I do not agree with the statement that header() writes to the bytestream. In PHP, if you already sent the headers, any header() results in an error (headers already sent).
I'm hesitant to merge this line change :D
There was a problem hiding this comment.
The error is exactly the reason, why I had to remove the line. If the line is present the preview image cannot be rendered, because of the error you mentioned
There was a problem hiding this comment.
Oh I see, makes sense. I guess my php.ini doesnt enable warnings on the page (you can disable that to the php error log where it belongs). Not nice that users can see whats going on :P
Can we at least move this line to the correct place?
|
Is everything clear now for this pull request? I have some widgets I'd like to contribute but I'll wait until this PR is accepted. So if you need more information please let me know |
|
Sorry I have way too many notifications in my github page and this fell off the radar. |
2 participants
No description provided.