2004199 - [GFX1-]: (mate) Wayland protocol error: wp_viewport#16096: error 2: source rectangle out of buffer bounds

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P3)

Description

[s.zharkoff]

Attachment: all_output.log.gz

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:147.0) Gecko/20100101 Firefox/147.0

Steps to reproduce:

Linux, Mate, wayfire. Open youtube, start video, scroll up-down several times while video playnig

Actual results:

Crash, wayland widget log attached

Expected results:

No crash

Comment 1

[s.zharkoff]

This issue was gone for a while and 1992198 was looking like a solution, but now it is back again. The most strange thing that it is pretty unstable - it can happen at very first scroll attempt or may take a while to happen. So I am noy sure if 1992198 actually fixed that or it was a kind of coinsidence and it simply was not happening after the bugfix landed. Now managed to repeat it again and capture all logs.

Comment 2

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[s.zharkoff]

Attachment: one more log - took longer to crash

Comment 4

[s.zharkoff]

Attachment: gdb backtrace

Comment 5

[s.zharkoff]

Attachment: log with extended info - D275210 applied

Comment 6

[s.zharkoff]

one more thing I found out - the scrolling crash 720p videos like this one

https://www.youtube.com/watch?v=JQArf1e5yDM&list=RDJQArf1e5yDM&start_radio=1

But 1080 FDH ones are OK. this one I can scroll back and forth may times no issues.

https://www.youtube.com/watch?v=p_IwFRtc5g4&list=RDp_IwFRtc5g4&start_radio=1

But as soon as I switch the reolution to 720 - I get crash almost immediately while scrolling.

Comment 7

[s.zharkoff]

not exactly the size of video matters but relations between viewport ratio where the video is shown and the video resolution. Changed window size and now appily crashing on 1920х1080.

1840х1035 viewport - videos 1080 fine, 480 fine
720 crash, 360 crash.

Looks like some rounding issue.

Comment 8

[Martin Stránský [:stransky] (ni? me)]

Is that HDR mode enabled?

Comment 9

[Martin Stránský [:stransky] (ni? me)]

So here we are:

[3892804.162] {Default Queue}  -> zwp_linux_buffer_params_v1#88.create_immed(new id wl_buffer#108, 1280, 720, 842094158, 0)

[Parent 1458754: Renderer]: D/WidgetWayland [7f5cbda69300]: WaylandSurface::SetViewPortSourceRectLocked(): [0.000000, 666.251953] -> [1279.440308 x 53.748047]
[3897773.282] {Default Queue}  -> wp_viewport#507.set_source(0.00000000, 666.25390625, 1279.44140625, 53.75000000)

coords are [1279.44140625 x 720.00390625]. I wonder if we should round that somehow.

Comment 10

[s.zharkoff]

in fact it does not matter if I enable HDR or not. Crashes anyway.
Also I've tried with labwc to exclude possible issue at wayfire side - no difference.

Comment 11

[s.zharkoff]

Also I've tried to change fractional scaling on/off in about:config - does not matter.

Comment 12

[s.zharkoff]

So looks like mismatch is between 1279.440308 and 1279.44140625. about 1/1000 of a pixel - does it really worth to calculate something that can not be displayed anyway? The minimal quant of of the screen is 1/3 of a pixel with sub-pixel rendering, but that's it. So rounding seems to be a good idea, not in the intermediate calculation as it can play a bad joke in calculation formulas, but after the calculation is done and pushing the data to gtk engine or compositor - dont know whoever is calling the wayland client call with mismatched numbers...

Comment 13

[s.zharkoff]

Attachment: round_try1.patch

Comment 14

[s.zharkoff]

applied this quick/dirty POC patch over autoland. Did not manage to crash while scrolling both 720 and 1080 videos. Not optimal for sure - using double math , while wl_fixed_from_double implements just some simple operations like adding and shifting.

Comment 15

[s.zharkoff]

still crashing - less but sometmes.

Comment 16

[s.zharkoff]

chanfed this *100, /100 to simple integer round(). Looks better. Tried to set 1.1 fracctional scaling on my display - no issue too. So weems that those decimals are not important for wp_viewport_set_sorce.

Comment 17

[s.zharkoff]

Attachment: round_try2.patch

Comment 18

[Martin Stránský [:stransky] (ni? me)]

We need to round x+width <= buffer_width here, so we'd need to round x or width or both somehow.

Comment 19

[s.zharkoff]

So far I've rounded all coords up to integer - x,y, width, height. Seems to be OK so far.

Comment 20

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 2004199 [Wayland] Use DesktopIntRect for viewporter source rect r?emilio

Comment 21

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 2004199 [Wayland/HDR] Round transformed buffer source coordinates r?emilio

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ecb3a4ee22be
https://hg.mozilla.org/integration/autoland/rev/43ca235314bf
[Wayland] Use DesktopIntRect for viewporter source rect r=emilio
https://github.com/mozilla-firefox/firefox/commit/ef8ac4ef5830
https://hg.mozilla.org/integration/autoland/rev/2c092c50a8ab
[Wayland/HDR] Round transformed buffer source coordinates r=emilio

Comment 23

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/43ca235314bf
https://hg.mozilla.org/mozilla-central/rev/2c092c50a8ab

Comment 24

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/43ca235314bf
https://hg.mozilla.org/mozilla-central/rev/2c092c50a8ab