Moodle
  1. Moodle
  2. MDL-11000

Profile does not support transparent PNG images properly

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 2.0
    • Component/s: General
    • Labels:
      None
    • Environment:
      GD 2
    • Rank:
      34772

      Description

      Uploading transparent PNG images to your profile causes the transparent colour to be made black.

      I implemented a fix for this for Mahara today, maybe Moodle is interested in it too . Patch for Mahara is here:

      http://git.catalyst.net.nz/gitweb?p=mahara.git;a=commitdiff;h=bfaa89d7ec77e2f47657783af277e78e6824a39a

      I think your gdlib.php, function imagecopybicubic is what needs to be patched, but it's after midnight here and I don't know the moodle codebase so well, so will leave the details to someone else

      1. patch.tar.gz
        0.5 kB
        Luigi Pinca

        Activity

        Hide
        Nigel McNie added a comment -

        First step - provide red licorice down payment.

        Show
        Nigel McNie added a comment - First step - provide red licorice down payment.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Uhm... I see... the key here is that, in Mahara, you save profile pictures as PNG, so transparency has sense. But in Moodle, we use to save those pictures as JPEG (not supporting transparency).

        Anyway, IMO, we should fill transparent pixels with white (instead of current-default black). Any opinion?

        Show
        Eloy Lafuente (stronk7) added a comment - Uhm... I see... the key here is that, in Mahara, you save profile pictures as PNG, so transparency has sense. But in Moodle, we use to save those pictures as JPEG (not supporting transparency). Anyway, IMO, we should fill transparent pixels with white (instead of current-default black). Any opinion?
        Hide
        Nigel McNie added a comment -

        Could Moodle be changed to save all pictures as PNGs?

        Or save PNG/GIF files as PNGs?

        To me, it makes sense that they're saved as the filetype they are (Mahara is not so good here, it makes everything PNG). That way no problems like this occur - and you get better file quality too with GIF/PNG images. But of course there are tradeoffs, such as in disk space used for caching the processed images (CPU time difference is negligible when things are being cached and given the amount of processing that goes into serving the average Moodle/Mahara page anyway).

        If not, then I agree: filling with white is better than black.

        Thanks for looking at this bug!

        Show
        Nigel McNie added a comment - Could Moodle be changed to save all pictures as PNGs? Or save PNG/GIF files as PNGs? To me, it makes sense that they're saved as the filetype they are (Mahara is not so good here, it makes everything PNG). That way no problems like this occur - and you get better file quality too with GIF/PNG images. But of course there are tradeoffs, such as in disk space used for caching the processed images (CPU time difference is negligible when things are being cached and given the amount of processing that goes into serving the average Moodle/Mahara page anyway). If not, then I agree: filling with white is better than black. Thanks for looking at this bug!
        Hide
        Tim Lock added a comment -

        Any chance of releasing a patch for Moodle 1.9 to handle allowing all image formats?

        I assume changes will be made to calls in lib/gdlib.php?

        Show
        Tim Lock added a comment - Any chance of releasing a patch for Moodle 1.9 to handle allowing all image formats? I assume changes will be made to calls in lib/gdlib.php?
        Hide
        Luigi Pinca added a comment -

        I wrote a small patch for the gdlib.php to keep the trasparency of png profile images.
        It has been tested on moodle 1.9.7 only.

        Show
        Luigi Pinca added a comment - I wrote a small patch for the gdlib.php to keep the trasparency of png profile images. It has been tested on moodle 1.9.7 only.
        Hide
        Luigi Pinca added a comment -

        The previous archive (tar.gz) holds a malformed patch file.
        This one is ok.

        Show
        Luigi Pinca added a comment - The previous archive (tar.gz) holds a malformed patch file. This one is ok.
        Hide
        Petr Škoda added a comment -

        silly question, is it storing png file with ".jpg" extension?

        Show
        Petr Škoda added a comment - silly question, is it storing png file with ".jpg" extension?
        Hide
        Petr Škoda added a comment -

        another silly question, does it work with IE6-7?

        Show
        Petr Škoda added a comment - another silly question, does it work with IE6-7?
        Hide
        Petr Škoda added a comment -

        The patch does not apply any more after the conversion to new file pool. Internally it would be possible to store the converted images in multiple formats and serve different image for different browser, the only problem is the required time to implement this and test on all browsers and.

        I am afraid this will not get into 2.0 because there are still very many blocker issues that need to be fixed by the next Tuesday

        Show
        Petr Škoda added a comment - The patch does not apply any more after the conversion to new file pool. Internally it would be possible to store the converted images in multiple formats and serve different image for different browser, the only problem is the required time to implement this and test on all browsers and. I am afraid this will not get into 2.0 because there are still very many blocker issues that need to be fixed by the next Tuesday
        Hide
        Luigi Pinca added a comment -

        @Petr.

        Yes it store a png file with jpg extension.
        Every profile image in moodle 1.9.7 is converted by default in jpg, so the system is looking for a file with jpg extension.
        What i did there was a "bad way" to overcome this problem.

        It works in IE7.
        In IE6 png files do not show transparency so the trasparent part may appear to be gray rather than transparent.

        Show
        Luigi Pinca added a comment - @Petr. Yes it store a png file with jpg extension. Every profile image in moodle 1.9.7 is converted by default in jpg, so the system is looking for a file with jpg extension. What i did there was a "bad way" to overcome this problem. It works in IE7. In IE6 png files do not show transparency so the trasparent part may appear to be gray rather than transparent.
        Hide
        Petr Škoda added a comment - - edited

        hmm, it is a tempting idea to ignore the IE6 and throw png on everybody, we can support both jpg and png files at the same time in pluginfile.php - we would just fetch all file infos in icon area and use png if found

        Anybody wants to work on this? the gdlib would preferably create .png files, and jpg if not possible, the the second change would be in pluginfile.php, fetch all files from file area and test if png exists, if not test if jpg exists, only then redirect to std image.....

        Show
        Petr Škoda added a comment - - edited hmm, it is a tempting idea to ignore the IE6 and throw png on everybody, we can support both jpg and png files at the same time in pluginfile.php - we would just fetch all file infos in icon area and use png if found Anybody wants to work on this? the gdlib would preferably create .png files, and jpg if not possible, the the second change would be in pluginfile.php, fetch all files from file area and test if png exists, if not test if jpg exists, only then redirect to std image.....
        Hide
        Petr Škoda added a comment -

        png now used for user avatars, jpg is used as fallback only
        thanks for the patch and this report

        Show
        Petr Škoda added a comment - png now used for user avatars, jpg is used as fallback only thanks for the patch and this report

          People

          • Votes:
            3 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: