Mir

[performance] Android overlays don't honour eglSwapInterval 0

Bug #1369763 reported by Alberto Aguirre
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Mir
Won't Fix
Medium
Kevin DuBois
mir-android-platform
New
Undecided
Unassigned
mir (Ubuntu)
Won't Fix
Medium
Unassigned

Bug Description

eglSwapInterval 0 does not work when overlays are enabled.

Both of these report the same fps:

MIR_CLIENT_PERF_REPORT=log ./mir_demo_server_basic --launch-client="./mir_demo_client_egltriangle" --disable-overlays false
MIR_CLIENT_PERF_REPORT=log ./mir_demo_server_basic --launch-client="./mir_demo_client_egltriangle -n" --disable-overlays false

Related branches

lp://staging/~mir-team/mir/more-performance-tests
Cemil Azizoglu (community): Approve
Alexandros Frantzis (community): Approve
PS Jenkins bot (community): Approve (continuous-integration)
Mir CI Bot: Approve (continuous-integration)
Alan Griffiths: Needs Information
Diff: 135 lines (+94/-10)
1 file modified
tests/performance-tests/test_glmark2-es2-mir.cpp (+94/-10)
lp://staging/~kdub/mir/fix-1369763
Daniel van Vugt: Approve
Brandon Schaefer (community): Approve
Mir CI Bot: Approve (continuous-integration)
Cemil Azizoglu (community): Approve
Alexandros Frantzis (community): Needs Fixing
Diff: 266 lines (+71/-2)
17 files modified
include/platform/mir/graphics/renderable.h (+1/-0)
src/include/server/mir/compositor/buffer_stream.h (+1/-0)
src/platform/symbols.map (+1/-0)
src/platforms/android/server/hwc_device.cpp (+2/-1)
src/platforms/android/utils/render_overlays.cpp (+5/-0)
src/platforms/android/utils/test_android_hardware_sanity.cpp (+4/-0)
src/server/compositor/stream.cpp (+5/-0)
src/server/compositor/stream.h (+1/-0)
src/server/graphics/software_cursor.cpp (+5/-0)
src/server/input/touchspot_controller.cpp (+6/-0)
src/server/scene/basic_surface.cpp (+5/-0)
tests/include/mir/test/doubles/fake_renderable.h (+5/-0)
tests/include/mir/test/doubles/mock_buffer_stream.h (+1/-0)
tests/include/mir/test/doubles/mock_renderable.h (+1/-0)
tests/include/mir/test/doubles/stub_buffer_stream.h (+4/-1)
tests/include/mir/test/doubles/stub_renderable.h (+13/-0)
tests/unit-tests/platforms/android/server/test_hwc_device.cpp (+11/-0)
lp://staging/~ci-train-bot/mir/mir-ubuntu-zesty-2180 (Merged)
lp://staging/~ci-train-bot/mir/mir-ubuntu-zesty-2209
lp://staging/~vanvugt/mir/fix-1657755
Mir CI Bot: Approve (continuous-integration)
Cemil Azizoglu (community): Approve
Kevin DuBois (community): Approve
Diff: 39 lines (+4/-7)
2 files modified
src/platforms/android/server/hwc_device.cpp (+1/-2)
tests/unit-tests/platforms/android/server/test_hwc_device.cpp (+3/-5)
Kevin DuBois (kdub)
Changed in mir:
status: New → Confirmed
Cemil Azizoglu (cemil-azizoglu)
Changed in mir:
importance: Medium → High
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Re: Overlays don't honor eglSwapInterval 0 (not implemented yet)

We don't actually have any code that uses swap interval 0 in production. Only demos. So it's not a big priority to implement this yet.

summary: - eglSwapInterval 0 does not work when overlays are enabled
+ Overlays don't honor eglSwapInterval 0 (not implemented yet)
tags: added: enhancement
Changed in mir:
importance: High → Medium
Daniel van Vugt (vanvugt)
Changed in mir:
status: Confirmed → Triaged
milestone: 0.8.0 → none
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote : Re: Overlays don't honor VSYNC

It's not that this is only a problem with swap_interval == 0. Even with 1 (i.e. without '-n'), we seem to be rendering at 67fps on _some_devices_ (wink, wink ...) when overlays are enabled. (Changed the title to more accurately reflect that.)

It may be a more fundamental problem in our rendering logic, so bumping priority back to high.

summary: - Overlays don't honor eglSwapInterval 0 (not implemented yet)
+ Overlays don't honor VSYNC
Changed in mir:
importance: Medium → High
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

@Cemil,

That's the vsync rate for that device.

Alberto Aguirre (albaguirre)
summary: - Overlays don't honor VSYNC
+ Overlays don't honor eglSwapInterval 0
Daniel van Vugt (vanvugt)
Changed in mir:
importance: High → Medium
Revision history for this message
Kevin DuBois (kdub) wrote : Re: Overlays don't honor eglSwapInterval 0

The unsupported combo mentioned in this bug has started manifesting itself in a new way with the introduction of BufferStream, client now repeatedly reports:

[1424295040.208033] <ERROR> MirBufferStream: Error processing incoming buffer error registering graphics buffer for client use

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Comment #4 is being discussed in bug 1441553. I've also mentioned there it's easy to reproduce with frame dropping clients.

And I think the original issue could be related to bug 1391261.

Daniel van Vugt (vanvugt)
tags: added: overlays
Daniel van Vugt (vanvugt)
summary: - Overlays don't honor eglSwapInterval 0
+ Android overlays don't honour eglSwapInterval 0
tags: added: performance
Daniel van Vugt (vanvugt)
summary: - Android overlays don't honour eglSwapInterval 0
+ [performance] Android overlays don't honour eglSwapInterval 0
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Interesting thought --

Since r3078 this bug is probably happening constantly in CI, so our automated performance tests will be bound to 60Hz instead of the default behaviour of glmark which is unbound.

So in a way our automated performance tests are now more sensitive to this bug than anything else :/

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

Fix committed into lp:mir at revision None, scheduled for release in mir, milestone 0.21.0

Changed in mir:
status: Triaged → Fix Committed
Daniel van Vugt (vanvugt)
Changed in mir:
status: Fix Committed → Triaged
Daniel van Vugt (vanvugt)
tags: removed: enhancement
Daniel van Vugt (vanvugt)
Changed in mir:
milestone: none → 0.25.0
assignee: nobody → Kevin Dubois (kevin-dubois05)
status: Triaged → In Progress
assignee: Kevin Dubois (kevin-dubois05) → Kevin DuBois (kdub)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

Fix committed into lp:mir at revision None, scheduled for release in mir, milestone 0.25.0

Changed in mir:
status: In Progress → Fix Committed
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Correction; that was just a workaround that landed (we now disable overlays if required). Still needs more work to fix properly.

Changed in mir:
milestone: 0.25.0 → none
status: Fix Committed → Triaged
Changed in mir (Ubuntu):
status: New → Triaged
importance: Undecided → Medium
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

Fix committed into lp:mir at revision None, scheduled for release in mir, milestone 1.0.0

Changed in mir:
status: Triaged → Fix Committed
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

That was not a fix that landed. It was the revert of the workaround for this bug.

Changed in mir:
status: Fix Committed → Triaged
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The Android platform was deleted from lp:mir at revision 4155.

Changed in mir:
status: Triaged → Won't Fix
Changed in mir (Ubuntu):
status: Triaged → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.