Add support for image diffs.
Review Request #4493 — Created Aug. 27, 2013 and submitted
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? |
 david
|
|
|
What if a particular view has some controls that use images? Should we have a specific class here? |
david
|
|
|
<=? How about === and we log an error if it's < |
david
|
-
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
-
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?
-
-
This is getting very boxy. Could we maybe move the "Download" buttons up next to the revision names somehow?
-
reviewboard/static/rb/js/views/imageReviewableView.js (Diff revision 1) What if a particular view has some controls that use images? Should we have a specific class here?
-
reviewboard/static/rb/js/views/imageReviewableView.js (Diff revision 1) <=? How about === and we log an error if it's <
Change Summary:
* Changed the conditional for loadsRemaining, and added an assert.
-
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
-
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
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