Fix for mir_performance_tests failure due to not cleaning up our displays

[mattkae]

How to test

• On an nvidia + intel platform, run:

sudo MIR_EXPERIMENTAL_HYBRID_GRAPHICS=1 MIR_SERVER_PLATFORM_RENDERING_LIBS=mir:eglstream-kms XDG_RUNTIME_DIR=/run/ ./bin/mir_performance_tests --gtest_filter=GLMark2Wayland.*

Expected output

LD_LIBRARY_PATH=./bin/../lib/
MIR_SERVER_PLATFORM_PATH=./bin/../lib/server-modules/
exec=./bin/mir_performance_tests.bin
Running main() from main.cpp
Note: Google Test filter = GLMark2Wayland.*
[==========] Running 2 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 2 tests from GLMark2Wayland
[ RUN      ] GLMark2Wayland.fullscreen
Saving server logs to: /tmp/GLMark2Wayland_fullscreen_server.log
libEGL warning: egl: failed to create dri2 screen
libEGL warning: egl: failed to create dri2 screen
libEGL warning: egl: failed to create dri2 screen
libEGL warning: egl: failed to create dri2 screen
Saving GLMark2 detailed results to: /tmp/GLMark2Wayland_fullscreen.log
[       OK ] GLMark2Wayland.fullscreen (11871 ms)
[ RUN      ] GLMark2Wayland.windowed
Saving server logs to: /tmp/GLMark2Wayland_windowed_server.log
libEGL warning: egl: failed to create dri2 screen
libEGL warning: egl: failed to create dri2 screen
libEGL warning: egl: failed to create dri2 screen
libEGL warning: egl: failed to create dri2 screen
Saving GLMark2 detailed results to: /tmp/GLMark2Wayland_windowed.log
[       OK ] GLMark2Wayland.windowed (11712 ms)
[----------] 2 tests from GLMark2Wayland (23584 ms total)

[----------] Global test environment tear-down
[==========] 2 tests from 1 test suite ran. (23584 ms total)
[  PASSED  ] 2 tests.

[codecov]

Codecov Report

Merging #3080 (a34f56b) into platform-API-merge (79622a9) will decrease coverage by 0.02%.
Report is 8 commits behind head on platform-API-merge.
The diff coverage is 48.48%.

@@                  Coverage Diff                   @@
##           platform-API-merge    #3080      +/-   ##
======================================================
- Coverage               75.94%   75.92%   -0.02%     
======================================================
  Files                    1058     1058              
  Lines                   74005    74028      +23     
======================================================
+ Hits                    56200    56206       +6     
- Misses                  17805    17822      +17     

Files	Coverage Δ
include/platform/mir/graphics/platform.h	85.29% <100.00%> (ø)
src/platforms/gbm-kms/server/kms/platform.cpp	23.83% <100.00%> (ø)
...latforms/renderer-generic-egl/platform_symbols.cpp	95.23% <100.00%> (ø)
src/platforms/wayland/platform_symbols.cpp	65.21% <ø> (ø)
src/platforms/x11/graphics/graphics.cpp	97.61% <ø> (ø)
src/platforms/x11/graphics/platform.cpp	90.32% <100.00%> (ø)
src/server/compositor/basic_screen_shooter.cpp	83.67% <100.00%> (ø)
src/server/graphics/default_configuration.cpp	79.83% <100.00%> (+0.25%)	⬆️
src/server/graphics/platform_probe.cpp	95.83% <100.00%> (ø)
src/server/input/vt_filter.cpp	6.66% <100.00%> (+1.58%)	⬆️
... and 13 more

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[AlanGriffiths]

For clarity, should we be testing against EGL_NO_STREAM_KHR (for example)?

And this looks like the same issue exists on main and maybe better applied there?

[RAOF]

For clarity, should we be testing against EGL_NO_STREAM_KHR (for example)?

Yeah. I can't find it offhand, but I'm pretty sure that there's at least one EGL_NO_FOO that's not 0.

[mattkae]

For clarity, should we be testing against EGL_NO_STREAM_KHR (for example)?

And this looks like the same issue exists on main and maybe better applied there?

We are already checking against EGL_NO_STREAM_KHR at construction. Unless I am misunderstanding you?

The original error occurs in DisplayBuffer::post() at

if (nv_stream(dpy).eglStreamConsumerAcquireAttribNV(dpy, output_stream, acquire_attribs) != EGL_TRUE)
...

[mattkae]

@AlanGriffiths @RAOF I can move this to main if you both think it makes sense. Unless the api-merge is going to main soon, in which case I will hold off

[AlanGriffiths]

This is what I meant.

Although, the tests are redundant: they test an invariant established in the constructor. (Which would be more obvious if the members were const.)