• 
      

    Add support for image diffs.

    Review Request #4493 — Created Aug. 27, 2013 and submitted

    Information

    Review Board
    master

    Reviewers

    Add support for image diffs.
    
    This greatly enhances the ImageReviewUI to support showing diffs between
    images. When generating a binary diff between images, the Review UI will
    provide several modes for viewing how the images changed.
    
    The first mode is the Two-Up mode, which just shows the two images
    side-by-side.
    
    The second is the Difference mode, which shows a color-based difference
    between the images. It's useful for spotting what pixels actually
    changed between images, and what remained the same.
    
    The third is the Split mode, which layers the two images and provides a
    slider for showing how much of the modified image is visible, vs. the
    original. The user can slide back-and-forth to see what changed between
    the images.
    
    The fourth is the Onion Skin mode, which layers the two images and
    provides a transparency slider. This slider will alter the transparency
    of the modified image. This makes it easy to see what pixels may have
    moved, or other subtle changes to the image.
    
    All modes allow commenting, and all comments are displayed on all modes.
    The comment thumbnail for image diffs will show the commented regions of
    the original and modified images side-by-side (similar to the Two-Up
    mode).
    Tested commenting on each of these modes.
    
    Tested commenting outside the region of one image or another, and made
    sure that didn't blow up (particularly when generating thumbnails).
    
    Tested that thumbnails showed up correctly in the review dialog and in
    published reviews.
    
    Tested diffing images of different sizes.
    
    Tested all the functionality for traditional image file attachments.
    
    I have *not* tested with very large images. I expect this to be painful
    to use, but have some plans to improve very large images in this review UI
    in general, so I'm okay with that for now.

    Description From Last Updated

    This is getting very boxy. Could we maybe move the "Download" buttons up next to the revision names somehow?

    daviddavid

    What if a particular view has some controls that use images? Should we have a specific class here?

    daviddavid

    <=? How about === and we log an error if it's <

    daviddavid
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/reviews/ui/image.py
          reviewboard/settings.py
        Ignored Files:
          reviewboard/templates/diffviewer/diff_file_fragment.html
          reviewboard/static/rb/css/reviews.less
          reviewboard/static/rb/css/diffviewer.less
          reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
          reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
          reviewboard/templates/reviews/ui/default.html
          reviewboard/static/rb/css/image-review-ui.less
          reviewboard/static/rb/js/views/imageReviewableView.js
          reviewboard/static/rb/js/models/imageReviewableModel.js
      
      
    2. 
        
    reviewbot
    1. This is a review from Review Bot.
        Tool: Pyflakes
        Processed Files:
          reviewboard/reviews/ui/image.py
          reviewboard/settings.py
        Ignored Files:
          reviewboard/templates/diffviewer/diff_file_fragment.html
          reviewboard/static/rb/css/reviews.less
          reviewboard/static/rb/css/diffviewer.less
          reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
          reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
          reviewboard/templates/reviews/ui/default.html
          reviewboard/static/rb/css/image-review-ui.less
          reviewboard/static/rb/js/views/imageReviewableView.js
          reviewboard/static/rb/js/models/imageReviewableModel.js
      
      
    2. 
        
    chipx86
    1. I'm not 100% happy with the name "Split." It's what things like Kaleidoscope and GitHub use, but I don't think it conveys the feature that well. I'm thinking of "Curtain." Is that better or worse?
      1. I'm ok with either.
    2. 
        
    david
    1. 
        
    2. Show all issues
      This is getting very boxy. Could we maybe move the "Download" buttons up next to the revision names somehow?
      1. Yeah, that's a future change. I think I mentioned this in the change that introduced the Download links, but they're temporary.
    3. Show all issues
      What if a particular view has some controls that use images? Should we have a specific class here?
      1. They can override render and not call the default in that case. The view that shows just one image does that, for example.
        
        It's a hypothetical right now, so I don't want to over-engineer it, but I think any view that wants non-default behavior can just not use the default behavior.
    4. Show all issues
      <=? How about === and we log an error if it's <
    5. 
        
    chipx86
    chipx86
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/reviews/ui/image.py
          reviewboard/settings.py
        Ignored Files:
          reviewboard/static/rb/css/reviews.less
          reviewboard/static/rb/css/diffviewer.less
          reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
          reviewboard/templates/reviews/ui/default.html
          reviewboard/static/rb/css/image-review-ui.less
          reviewboard/static/rb/js/views/imageReviewableView.js
          reviewboard/static/rb/js/models/imageReviewableModel.js
      
      
    2. 
        
    reviewbot
    1. This is a review from Review Bot.
        Tool: Pyflakes
        Processed Files:
          reviewboard/reviews/ui/image.py
          reviewboard/settings.py
        Ignored Files:
          reviewboard/static/rb/css/reviews.less
          reviewboard/static/rb/css/diffviewer.less
          reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
          reviewboard/templates/reviews/ui/default.html
          reviewboard/static/rb/css/image-review-ui.less
          reviewboard/static/rb/js/views/imageReviewableView.js
          reviewboard/static/rb/js/models/imageReviewableModel.js
      
      
    2. 
        
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed