Welcome to the Splat alpha!

We wouldn't really be able to make this work with Git the way you'd hope, as Review
Board wouldn't necessarily have access to the modifications.

Today, you can upload screenshots (really, any image) much like you can with diffs
and people can review that by clicking and dragging on the screenshot to comment on
that region. Would that suffice?

• Status:

  +	NeedInfo

The typical diff from git gives you something like:

diff --git a/docs/sdk/htmltable.png b/docs/sdk/htmltable.png
new file mode 100644
index 0000000..60617a2
Binary files /dev/null and b/docs/sdk/htmltable.png differ

Seems like there's enough there that reviewboard could ask git for the blob:

$git-cat-file blob 60617a2 > /tmp/z; file /tmp/z
/tmp/z: PNG image data, 813 x 142, 8-bit/color RGB, non-interlaced

I've noticed that reviewboard already does cat-file (our reviewboard server polls git every 5 minutes, and 
sometimes refuses to accept patches against versions that don't yet exist), so seems like it could do the same 
here?

Am I missing something?

Thanks!

We can only get the original, not the modifications. There's no way to see what
changed, which makes it totally useless. There's also no way of making it work with
other types of repositories.

What we'd have to do is provide support in post-review for grabbing these modified
binary files, figuring out the correct mimetypes, and uploading them. That might
happen in a future release, but is not as easy as it may sound.

Seems fair to relegate this sort of feature to post-review.  For images used in web development, just seeing old 
and new (both of which may be accessible using git-cat-file, if using git) is often good enough, and that's 
probably the right thing to upload.

• Status:

  -	NeedInfo
  +	New

• Tags:

  +	Component-DiffViewer

I don't see why you can't get the uploaded patch-file to include the binary diff.

<Edit image>
# hg diff test.png --text > my-review.patch
# hg revert test.png
# file test.png
test.png: PNG image, 310 x 320, 8-bit/color RGBA, non-interlaced
# patch -p1 < my-review.patch
patching file test.png
# file test.png
test.png: PNG image, 320 x 281, 8-bit/color RGB, non-interlaced

Seems to me that reviewboard will have all the info it needs.

I've found this ticket by searching for a way to review the image diffs which, unfortunately, still doesn't seem possible even with the latest version and I admit I don't understand what exactly the problem with getting the old and new contents of the file is. At least with git, git diff --binary can be used to include the full image contents in the diff, so the data could definitely be provided to ReviewBoard.
It seems really a pity that there is already an image review tool, but it can be only used for the attachments and not the files included in the review.

Hello! It's Juan!
This would be super helpful for us GUI/design guys.

Hi Juan,
Review Board does do diffs of images that are attached to review requests, which is probably more what you want (since I assume you're not actually committing images to version control?)
See https://www.reviewboard.org/docs/manual/2.5/users/reviews/reviewing-images/ and https://www.reviewboard.org/docs/manual/2.5/users/reviews/reviewing-files/#reviewing-file-attachments

Actually, we do check into version control the icons and other images that show up on the final product. 
The diffs/comments on attached images are super useful and we attach screenshots on every GUI review. Would be great to do something similar to committed images too.

There's a lot of work that went into the tree for allowing review of images and other file types in diffs, but we never put together all the actual support for uploading files (as they're not contained within diffs themselves, except on Git in a way that's not efficient enough for our usage). I want to get that into 4.0 (3.0 is currently under development and we're working to finish off the last of the features).

Thanks! 

If there is no way to get the images automatically from the diff/repository, there will be little difference to attaching manually the new image files to the review (like we do today).

It will fetch from the repository and from your local change. That's the part that's just not written yet, and the reason we haven't made the rest of it public yet. It will require uploading via RBTools, though.