Enable visual image differences in Gerrit (was: Install gerrit image-diff plugin)
Open, Needs TriagePublic
Actions

Assigned To

None

Authored By

	hashar
	Jul 6 2023, 8:45 PM

Description

Upstream patches:

Mentioned by Tyler during a meeting: add the ability to diff images in Gerrit. Upstream has the image-diff JavaScript plugin for it. See README/screenshots at https://github.com/GerritCodeReview/plugins_image-diff#image-diff

That requires an inline build with bazel build plugins/image-diff to install the dependencies.

We also found out a diff is included inside Gerrit core itself albeit hidden behind a feature flag (UiFeature__new_image_diff_ui), this approach is privileged instead of relying on the plugin.

The plugin or experiment integrates the Resemble.js library and can be tested against:

Image from ResembleJS demo site: https://gerrit.wikimedia.org/r/c/test/gerrit-ping/+/1051109/1..2/People.jpg

https://gerrit.wikimedia.org/r/c/operations/mediawiki-config/+/902211/1/static/images/project-logos/dkwikimedia.png which slightly change the Danmark Wikimedia logo:

Before	After

Which then let you see the differences (using cyan but that can be changed via a color picker):

Make the images transparent to better see the diff:

Or go in gray mode:

There are some CSS glitches though but that do not seem too worry some.

Details

Related Changes in Gerrit:

Subject	Repo	Branch	Lines +/-
Test image differences	test/gerrit-ping	master	+0 -0
gerrit: enable "new" image diff UI	operations/puppet	production	+3 -0
Update Gerrit 3.10 snapshot	operations/software/gerrit	deploy/wmf/stable-3.10	+14 -14
Handle image-diff external dependencies	operations/software/gerrit	wmf/stable-3.10	+21 -0
Add image-diff JavaScript plugin (take 2)	operations/software/gerrit	deploy/wmf/stable-3.10	+346 -0
Revert "Add image-diff JavaScript plugin"	operations/software/gerrit	deploy/wmf/stable-3.10	+0 -345
Add image-diff JavaScript plugin	operations/software/gerrit	deploy/wmf/stable-3.10	+345 -0
Add image-diff plugin	operations/software/gerrit	wmf/stable-3.10	+31 -11

Customize query in gerrit

Related Objects

Mentioned In: T373897: Upgrade to Gerrit 3.10.2
T367029: "Press c to comment" is placed incorrectly when using Firefox 126 and 128 on macOS
Mentioned Here: T367029: "Press c to comment" is placed incorrectly when using Firefox 126 and 128 on macOS
T355931: Gerrit 3.7: copyable Change URLs broken

Event Timeline

hashar created this task.Jul 6 2023, 8:45 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 6 2023, 8:45 PM

+ @Paladox since I am pretty sure you talked about image-diff in the future and you have the relevant expertise for PolyGerrit plugins :)

If course I had to send a few patches due to the plugin depending on Resemble.JS 3.2.4 which hard depend on canvas → cairo → some missing system libs. I have send some fixes to upstream:

https://gerrit-review.googlesource.com/c/plugins/image-diff/+/378997/ Improve README.md build instructions and more
https://gerrit-review.googlesource.com/c/plugins/image-diff/+/378998/ Update resemble Resemble 3.2.4 > 4.0.0

hashar updated the task description. (Show Details)Jul 7 2023, 4:01 PM

The patches have been merged upstream thanks to @Paladox

Change #1050334 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@wmf/stable-3.10] Add image-diff plugin

https://gerrit.wikimedia.org/r/1050334

gerritbot added a project: Patch-For-Review.Jun 27 2024, 12:06 PM

Change #1050395 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@deploy/wmf/stable-3.10] Add image-diff plugin

https://gerrit.wikimedia.org/r/1050395

Change #1050334 merged by jenkins-bot:

[operations/software/gerrit@wmf/stable-3.10] Add image-diff plugin

https://gerrit.wikimedia.org/r/1050334

Change #1050395 merged by jenkins-bot:

[operations/software/gerrit@deploy/wmf/stable-3.10] Add image-diff JavaScript plugin

https://gerrit.wikimedia.org/r/1050395

Mentioned in SAL (#wikimedia-operations) [2024-06-27T15:04:32Z] <hashar@deploy1002> Started deploy [gerrit/gerrit@9652bc3]: Add image-diff JavaScript plugin - T341291

Mentioned in SAL (#wikimedia-operations) [2024-06-27T15:04:40Z] <hashar@deploy1002> Finished deploy [gerrit/gerrit@9652bc3]: Add image-diff JavaScript plugin - T341291 (duration: 00m 07s)

Change #1050399 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@deploy/wmf/stable-3.10] Revert "Add image-diff JavaScript plugin"

https://gerrit.wikimedia.org/r/1050399

Change #1050399 merged by jenkins-bot:

[operations/software/gerrit@deploy/wmf/stable-3.10] Revert "Add image-diff JavaScript plugin"

https://gerrit.wikimedia.org/r/1050399

Maintenance_bot removed a project: Patch-For-Review.Jun 27 2024, 3:31 PM

Change #1050420 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@wmf/stable-3.10] Handle image-diff external dependencies

https://gerrit.wikimedia.org/r/1050420

gerritbot added a project: Patch-For-Review.Jun 27 2024, 4:36 PM

Change #1050428 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@deploy/wmf/stable-3.10] Add image-diff JavaScript plugin (take 2)

https://gerrit.wikimedia.org/r/1050428

Change #1050428 merged by jenkins-bot:

[operations/software/gerrit@deploy/wmf/stable-3.10] Add image-diff JavaScript plugin (take 2)

https://gerrit.wikimedia.org/r/1050428

Mentioned in SAL (#wikimedia-operations) [2024-06-27T17:13:34Z] <hashar@deploy1002> Started deploy [gerrit/gerrit@8c6ae73]: Add image-diff JavaScript plugin (take 2) - T341291

Mentioned in SAL (#wikimedia-operations) [2024-06-27T17:13:41Z] <hashar@deploy1002> Finished deploy [gerrit/gerrit@8c6ae73]: Add image-diff JavaScript plugin (take 2) - T341291 (duration: 00m 07s)

Change #1050420 merged by jenkins-bot:

[operations/software/gerrit@wmf/stable-3.10] Handle image-diff external dependencies

https://gerrit.wikimedia.org/r/1050420

Maintenance_bot removed a project: Patch-For-Review.Jun 27 2024, 5:30 PM

when looking at the test file https://gerrit.wikimedia.org/r/c/operations/mediawiki-config/+/902211/1/static/images/project-logos/dkwikimedia.png , Gerrit retrieves the image using
https://gerrit.wikimedia.org/r/r/changes/operations%2Fmediawiki-config~902211/revisions/1/files/static%2Fimages%2Fproject-logos%2Fdkwikimedia.png/content . That has two /r which causes some HTML error to be sent and thus an invalid image. That is similar to T355931 which also was mishandling our URL prefix.

Also, I am confused since Gerrit 3.5 has code to triggers ResembleJS https://gerrit-review.googlesource.com/c/gerrit/+/310486 . Then that refers to a <gr-image-viewer> element and I can't find it :/

Apparently the image diff had been broken for quite a while and @Paladox has a fix for the /r/r in URL with: https://gerrit-review.googlesource.com/c/gerrit/+/431757

The ResembleJS code I have found in Gerrit core apparently comes from an experiment made around 2021 but which is behind a feature flag https://gerrit-review.googlesource.com/c/gerrit/+/296987/1/polygerrit-ui/app/services/flags/flags.ts :

NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui'

My guess is we could turn it on, polish up the code in Gerrit core itself and aim for having it ready for Gerrit 3.11 release. That would then make the image-diff plugin obsolete.

Change #1050595 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@deploy/wmf/stable-3.10] Update Gerrit 3.10 snapshot

https://gerrit.wikimedia.org/r/1050595

gerritbot added a project: Patch-For-Review.Jun 28 2024, 12:24 PM

Change #1050595 merged by jenkins-bot:

[operations/software/gerrit@deploy/wmf/stable-3.10] Update Gerrit 3.10 snapshot

https://gerrit.wikimedia.org/r/1050595

Mentioned in SAL (#wikimedia-operations) [2024-06-28T12:55:19Z] <hashar@deploy1002> Started deploy [gerrit/gerrit@0db053e]: Upgrade Gerrit 3.10.0-32-gf77960412e to 3.10.0-71-gf6e9431fff - T367029 T341291

Mentioned in SAL (#wikimedia-operations) [2024-06-28T12:55:35Z] <hashar@deploy1002> Finished deploy [gerrit/gerrit@0db053e]: Upgrade Gerrit 3.10.0-32-gf77960412e to 3.10.0-71-gf6e9431fff - T367029 T341291 (duration: 00m 09s)

NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui'

If I craw through the DOM to reach the <gr-diff> element then enable the feature via the console using: $0.useNewImageDiffUi = true I get the new UI:

gerrit_useNewImageDiffUi.png (634×959 px, 70 KB)

A checkbox highlights the differences, that is powered by ResembleJS

gerrit_useNewImageDiffUi_highlight_differences.png (634×959 px, 63 KB)

And there is even a blinking feature to alternate between the versions:

Maintenance_bot removed a project: Patch-For-Review.Jun 28 2024, 1:31 PM

Change #1050614 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/puppet@production] gerrit: enable "new" image diff UI

https://gerrit.wikimedia.org/r/1050614

gerritbot added a project: Patch-For-Review.Jun 28 2024, 1:50 PM

Daimona awarded a token.Jun 28 2024, 1:59 PM

We can enable the feature on Monday when there is plenty of time to deal with the potential aftermaths ;)

Change #1050614 merged by Jelto:

[operations/puppet@production] gerrit: enable "new" image diff UI

https://gerrit.wikimedia.org/r/1050614

Maintenance_bot removed a project: Patch-For-Review.Jul 1 2024, 9:30 AM

Mentioned in SAL (#wikimedia-releng) [2024-07-01T11:09:00Z] <hashar> Restarting Gerrit to apply configuration change | T341291

Change #1051109 had a related patch set uploaded (by Hashar; author: Hashar):

[test/gerrit-ping@master] Test image differences

https://gerrit.wikimedia.org/r/1051109

gerritbot added a project: Patch-For-Review.Jul 1 2024, 11:39 AM

hashar updated the task description. (Show Details)Jul 1 2024, 11:42 AM

Change #1051109 abandoned by Hashar:

[test/gerrit-ping@master] Test image differences

Reason:

https://gerrit.wikimedia.org/r/1051109

Maintenance_bot removed a project: Patch-For-Review.Jul 1 2024, 12:31 PM

Change #1051109 restored by Paladox:

[test/gerrit-ping@master] Test image differences

https://gerrit.wikimedia.org/r/1051109

Change #1051109 abandoned by Paladox:

[test/gerrit-ping@master] Test image differences

Reason:

https://gerrit.wikimedia.org/r/1051109

The Gerrit core built-in UiFeature__new_image_diff_ui experiment generates a strict difference so that even a very slight change to colors/alpha/brightness cause the diff to be marked. There is a note left in the code about making it configurable which I have proposed upstream ( https://gerrit-review.googlesource.com/c/gerrit/+/432057 ).

newdiff_hidden_radio.png (574×889 px, 131 KB)

default view, Highlight differences is disabled, radio buttons are hidden

newdiff_enabled.png (574×889 px, 142 KB)

Highlight differences is enabled, default to ignore "Only substantial differences"

newdiff_all_differences.png (574×889 px, 67 KB)

"All differences" selected even the slightest change is shown

Ignore anti aliasing

Ignore colors

Ignore alpha

hashar attached a referenced file: F56174818: newdiff_hidden_radio.png. (Show Details)Jul 3 2024, 8:53 AM

hashar attached a referenced file: F56145742: newdiff_alpha.png. (Show Details)

hashar attached a referenced file: F56145644: newdiff_ignore_colors.png. (Show Details)

hashar attached a referenced file: F56145547: newdiff_antialiasing.png. (Show Details)

hashar attached a referenced file: F56145479: newdiff_nothing.png. (Show Details)

hashar attached a referenced file: F56145226: newdiff_enabled.png. (Show Details)

hashar attached a referenced file: F56145123: newdiff.png. (Show Details)

After renaming the options:

newdiff_options_reworded.png (574×889 px, 142 KB)

hashar attached a referenced file: F56217014: newdiff_ignore_alpha.png. (Show Details)Jul 4 2024, 8:00 AM

hashar attached a referenced file: F56217003: newdiff_ignore_colors_changes.png. (Show Details)

hashar attached a referenced file: F56216998: newdiff_ignore_anti-aliasing.png. (Show Details)

hashar attached a referenced file: F56216982: newdiff_all_differences.png. (Show Details)

hashar attached a referenced file: F56216962: newdiff_enabled.png. (Show Details)

hashar renamed this task from Install gerrit image-diff plugin to Enable visual image differencein Gerrit (was Install gerrit image-diff plugin).Jul 4 2024, 9:52 AM

hashar updated the task description. (Show Details)

hashar mentioned this in T373897: Upgrade to Gerrit 3.10.2.Sep 3 2024, 4:02 PM

The patches I wrote got reverted by upstream. I was upgrading Resemble.JS which is fine in the Open Source version of Gerrit but went to conflict within Google monorepository. The code I wrote should thus be adjusted to support both the old version of the library AND the newer version of it which would allow Google to import my patches internally.

IIRC that is https://gerrit-review.googlesource.com/c/gerrit/+/432142 which switches to a new compare() which accepts an object for configuration ({ ignore: 'nothing'}) while the old version uses chainable methods (.compareTo().ignoreNothing()). In https://gerrit-review.googlesource.com/c/gerrit/+/432057/10/polygerrit-ui/app/embed/diff/gr-diff-image-viewer/gr-image-viewer.ts , I have mapped a UI radio group value to the ignore value and the old API doesn't even support all those ignores iirc.

So it is not that complicated to implement, but still requires some time :-]

I might want to also reach out to Google engineers and ask them to upgrade Resemble.JS internally, but I imagine it would be a very low priority for them.

Aklapper renamed this task from Enable visual image differencein Gerrit (was Install gerrit image-diff plugin) to Enable visual image differences in Gerrit (was: Install gerrit image-diff plugin).Oct 30 2024, 12:30 PM

Aklapper removed a project: Patch-For-Review.Feb 5 2025, 5:35 PM

	F56217003: newdiff_ignore_colors_changes.png
	Jul 4 2024, 7:59 AM

	F56216998: newdiff_ignore_anti-aliasing.png
	Jul 4 2024, 7:59 AM

Enable visual image differences in Gerrit (was: Install gerrit image-diff plugin)Open, Needs TriagePublicActions

Description

Details

Related Objects

Event Timeline

Enable visual image differences in Gerrit (was: Install gerrit image-diff plugin)
Open, Needs TriagePublic
Actions