Git Merge Request #281: Allow bitmap images for screenshots (merged)

Repo status: analyzing...

Merging...

Merged

Something went wrong. Please, merge manually

Checking if merge is possible...

Something went wrong. Please, merge manually

Shalitha Suranga wants to merge 13 commits from /u/shalithasuranga/allura/ to master, 2018-09-27

This fixes #2578 issue

Commit Date  
[f9dc03] (bitmapfix) by Shalitha Suranga Shalitha Suranga

[#2578] Clean up some codes

2018-09-27 05:41:40 Tree
[2f8b75] by Shalitha Suranga Shalitha Suranga

[#2578] Fix random number appending for bmps

2018-09-26 08:23:33 Tree
[debc2b] by Shalitha Suranga Shalitha Suranga

[#2578] Remove dev print log

2018-09-26 08:18:50 Tree
[957fb0] by Shalitha Suranga Shalitha Suranga

[#2578] Convert bmp to png

2018-09-26 06:18:16 Tree
[76768e] by Shalitha Suranga Shalitha Suranga

[#2578] Set convert bmp parameter as True

2018-09-26 06:17:19 Tree
[ea1e60] by Shalitha Suranga Shalitha Suranga

Merge branch 'bitmapfix' of https://forge-allura.apache.org/git/u/shalithasuranga/allura into bitmapfix

2018-09-26 05:46:28 Tree
[b49aca] by Shalitha Shalitha

[#2578] Fix merge conflicts

2018-09-25 16:46:02 Tree
[af087b] by Shalitha Shalitha

Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/allura into bitmapfix

2018-09-25 16:42:44 Tree
[f64519] by Shalitha Shalitha

[#2578] Add Convert bmp to jpeg screenshots when uploading

2018-09-25 16:42:03 Tree
[bf73c3] (sdelconfirm) by Shalitha Suranga Shalitha Suranga

[#8238] Display js confirm when delete a screenshot

2018-09-21 05:39:37 Tree
[437b98] by Shalitha Suranga Shalitha Suranga

[#8238] Add class name to detect via Jquery

2018-09-21 05:38:46 Tree
[4ed6fb] by Shalitha Shalitha

Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/allura into bitmapfix

2018-09-17 17:24:01 Tree
[ebaed6] by Shalitha Shalitha

Add bmp support

2018-09-17 17:22:19 Tree

Discussion

  • Dave Brondsema

    Dave Brondsema - 2018-09-17

    BMP files aren't very efficient in terms of file size, so I don't think we would want to keep them as bitmap and serve them that way. If we wanted to allow them for uploads, they should be converted to PNG I think.

     
  • Yeah usually bitmaps are heavy in size. Better to convert them to png or jpg

     
  • Hi.
    I pushed few commits .. added jpeg conversion when file format is bmp. png thumb looked somewhat heavy. png or jpg ?

     
  • Dave Brondsema

    Dave Brondsema - 2018-09-25

    Works well. I think it'll depend on the exact image if PNG or JPG turns out better, but we'll just have to pick one. JPG can be lossy when it compresses, so I would probably prefer PNG. Even if its a bit heavier its still a lot better than BMP :)

    This automatically converts bitmaps when they are attached to tickets, comments, etc too. At first I thought that was great, but then I thought of cases where people want to attach a specific file, perhaps as an example or test case for something and they want that exact file to by attached, and not converted.

    So how about adding a parameter like convert_bmp=False to save_image() so that each place that uses it can decide if it makes sense to convert bmp files. And then add_screenshot and save_icon could set it to True. Or something along those lines.

     
  • Yeah great.. we will go with PNG then. wow I exactly used convert_bmp boolean before and just remove from commit becasue it looked not very useful because we don't want to save pure bmp anywhere.

    I will push some commits with above modifications

     
  • Hi.. Dave

    I made few commits by adding these this as per below

    • Convert bmp to png
    • Fix random number appending for bmp duplicates
    • Added boolean parameter
     
  • Dave Brondsema

    Dave Brondsema - 2018-09-26

    Hi Shalitha,

    Looking good. Just some cleanup/improvement suggestions now:

    Instead of re.compile('(.*)\.(.*)') and checking for match groups, os.path.basename or os.path.splitext are much nicer to use. Easier to understand what is happening.

    save_thumbnail doesn't check the convert_bmp parameter - although this doesn't seem to cause any problems, and acutally is good, it makes it so if you upload a BMP to a ticket or forum, it makes a thumbnail that is PNG and keeps the original as BMP :) So how about just removing the parameter from the save_thumbnail method theN?

    Thanks :D

     
  • Hi..

    os.path.splitext now used and removed boolean parameter also

    Thanks

     
  • Dave Brondsema

    Dave Brondsema - 2018-09-27
    • Status: open --> merged
     

Log in to post a comment.