platforms/renderer-generic-egl: Support CPUAddressableDisplayProvider #3095

RAOF · 2023-10-25T04:37:46Z

RAOF · 2023-10-25T04:38:14Z

(Don't yet know if it actually fixes that; putting up a PR so that I can get a snap to test 😉 )

RAOF · 2023-10-25T06:41:54Z

Hm, no, doesn't fix it, because eglGeyDisplay(EGL_DEFAULT_DISPLAY) only tries X11 (on mesa).

Time to pull out EGL_MESA_platform_surfaceless

mattkae · 2023-10-25T21:47:36Z

Hm, no, doesn't fix it, because eglGeyDisplay(EGL_DEFAULT_DISPLAY) only tries X11 (on mesa).

Time to pull out EGL_MESA_platform_surfaceless

The following makes us not fail right away:

auto dpy = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA, EGL_DEFAULT_DISPLAY, NULL);

But you'll start getting some funny errors like:

Maybe you know something I don't know there :) This post had some good info

Update
If you change the egl config dance to the following in both rendering_platform.cpp and buffer_allocator.cpp, it works:

if (eglChooseConfig(dpy, config_attr, &cfg, 1, &num_configs) != EGL_TRUE)
{
    BOOST_THROW_EXCEPTION((mg::egl_error("Failed to find any matching EGL config")));
}

auto ctx = eglCreateContext(dpy, EGL_NO_CONFIG_KHR, share_context.value_or(EGL_NO_CONTEXT), context_attr);
if (ctx == EGL_NO_CONTEXT)
{
    BOOST_THROW_EXCEPTION(mg::egl_error("Failed to create EGL context"));
}

@RAOF Kwin checks if the num_configs is zero and if it is, it uses EGL_NO_CONFIG_KHR and instead of cfg. That looks to be the answer? I will push a commit, but feel free to revert if it looks wrong 😉

…lable

codecov · 2023-10-25T22:26:16Z

Codecov Report

Merging #3095 (898dab5) into main (4a57bc6) will decrease coverage by 0.12%.
Report is 36 commits behind head on main.
The diff coverage is 36.20%.

@@            Coverage Diff             @@
##             main    #3095      +/-   ##
==========================================
- Coverage   78.43%   78.31%   -0.12%     
==========================================
  Files        1073     1074       +1     
  Lines       74717    74779      +62     
==========================================
- Hits        58602    58564      -38     
- Misses      16115    16215     +100

Files	Coverage Δ
include/platform/mir/graphics/egl_extensions.h	`90.90% <ø> (ø)`
src/platforms/virtual/platform.cpp	`61.70% <ø> (+4.98%)`	⬆️
...sts/unit-tests/platforms/virtual/test_platform.cpp	`100.00% <ø> (ø)`
...latforms/renderer-generic-egl/buffer_allocator.cpp	`61.49% <50.00%> (-0.15%)`	⬇️
src/platform/graphics/egl_extensions.cpp	`45.08% <63.63%> (+1.83%)`	⬆️
...tforms/renderer-generic-egl/rendering_platform.cpp	`71.11% <44.44%> (-9.18%)`	⬇️
...latforms/renderer-generic-egl/platform_symbols.cpp	`54.05% <0.00%> (-41.19%)`	⬇️

... and 29 files with indirect coverage changes

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

…ported” function. Because we have this code *everywhere*.

… them Also, use the `EGL_EXT_platform_base` extension rather than implicitly relying upon EGL 1.5 entry points. There's at least one EGL driver we want to support that doesn't support EGL 1.5.

RAOF · 2023-10-26T07:19:31Z

There we go, that should be good to go. I just:

Fixed up the eglGetPlatformDisplay usage - we need to go through the extension, not through the EGL 1.5 entrypoint (because at least one driver we want to support does not provide EGL 1.5 entrypoints)
Added a bunch of extension checks, to validate that what we're doing is right.

Dunno why this links locally fine, but we do indeed need to resolve those!

AlanGriffiths

Looks like a good approach, but there are a few bits to tidy up

include/platform/mir/graphics/egl_extensions.h

src/platform/graphics/egl_extensions.cpp

Saviq · 2023-10-26T10:26:11Z

So this isn't yet stable:

$ MIR_SERVER_PLATFORM_DISPLAY_LIBS=mir:virtual MIR_SERVER_PLATFORM_RENDERING_LIBS=mir:egl-generic MIR_SERVER_VIRTUAL_OUTPUT=1280x1024 ./bin/mir_performance_tests

I'm getting this on the CompositorPerformance test:

[ RUN      ] CompositorPerformance.regression_test_1563287
Saving server logs to: /tmp/CompositorPerformance_regression_test_1563287_server.log
/home/michal/dev/MirServer/mir/tests/performance-tests/test_compositor.cpp:102: Failure
Expected: (compositor_fps) >= (0), actual: -1 vs 0
/home/michal/dev/MirServer/mir/tests/performance-tests/test_compositor.cpp:103: Failure
Expected: (compositor_render_time) > (0), actual: -1 vs 0
[  FAILED  ] CompositorPerformance.regression_test_1563287 (14611 ms)

And this, once…

[ RUN      ] HostedGLMark2Wayland.windowed                                                                                         
Saving host output to: /tmp/HostedGLMark2Wayland_windowed_host.log
Saving server logs to: /tmp/HostedGLMark2Wayland_windowed_server.log
Mir fatal error: Critical error in Wayland platform: /home/michal/dev/MirServer/mir/src/platforms/wayland/display.cpp(250): Throw in function void mir::graphics::wayland::Display::run()
Dynamic exception type: boost::wrapexcept<std::system_error>
std::exception::what: Failed to read Wayland events: Broken pipe

Neither are a blocker, FWIW.

AlanGriffiths · 2023-10-26T11:28:37Z

Expected: (compositor_render_time) > (0), actual: -1 vs 0

This is because we don't get any output from -compositor-report=log: #3101

Saviq

Neither are a blocker, FWIW.

I've confirmed both on main, so good with me!

AlanGriffiths · 2023-10-26T13:03:06Z

Expected: (compositor_render_time) > (0), actual: -1 vs 0

This is because we don't get any output from -compositor-report=log: #3101

But now I do:

$ grep compositor: /tmp/CompositorPerformance_regression_test_1563287_server.log
[2023-10-26 14:00:36.827435] <information> compositor: Started
[2023-10-26 14:00:48.289107] <information> compositor: Stopped

Was I mad then? Or am I mad now?

…r-from-virtual-platform [mir:virtual] drop GenericEGLDisplayProvider

platforms/renderer-generic-egl: Support CPUAddressableDisplayProvider

ae44c18

RAOF marked this pull request as draft October 25, 2023 04:38

Falling back to EGL_PLATFORM_SURFACELESS_MESA if a display isn't avai…

679cca8

…lable

mattkae force-pushed the allow-llvmpipe-rendering branch from b126c08 to 679cca8 Compare October 25, 2023 21:58

RAOF added 3 commits October 26, 2023 18:04

graphics/EGLExtensions: Factor out a quick “check if extension is sup…

59bb6c5

…ported” function. Because we have this code *everywhere*.

platforms/renderer-generic-egl: Check for EGL extensions before using…

1ff3685

… them Also, use the `EGL_EXT_platform_base` extension rather than implicitly relying upon EGL 1.5 entry points. There's at least one EGL driver we want to support that doesn't support EGL 1.5.

Whitespace cleanup

94a4f85

RAOF marked this pull request as ready for review October 26, 2023 07:15

platform: Export new has_*_extension entrypoints

7fc304b

Dunno why this links locally fine, but we do indeed need to resolve those!

AlanGriffiths requested changes Oct 26, 2023

View reviewed changes

include/platform/mir/graphics/egl_extensions.h Outdated Show resolved Hide resolved

src/platform/graphics/egl_extensions.cpp Outdated Show resolved Hide resolved

src/platform/graphics/egl_extensions.cpp Outdated Show resolved Hide resolved

Tidy up

867f03c

The virtual platform should not offer a GenericEGLDisplayProvider

54c2ed1

Saviq approved these changes Oct 26, 2023

View reviewed changes

Drop corresponding tests

cfbd82e

AlanGriffiths enabled auto-merge October 26, 2023 13:37

This comment was marked as off-topic.

Sign in to view

Removing another unnecessary GenericEGLDisplayProvider test

a50e8ad

Saviq mentioned this pull request Oct 26, 2023

Display scaling is broken post platform refactor #3103

Closed

Saviq requested a review from AlanGriffiths October 26, 2023 17:55

Saviq disabled auto-merge October 26, 2023 17:56

Saviq enabled auto-merge October 26, 2023 17:56

Merge pull request #3100 from MirServer/drop-GenericEGLDisplayProvide…

898dab5

…r-from-virtual-platform [mir:virtual] drop GenericEGLDisplayProvider

Saviq disabled auto-merge October 26, 2023 18:32

Saviq enabled auto-merge October 26, 2023 18:33

Saviq disabled auto-merge October 27, 2023 05:34

Saviq merged commit a558e9c into main Oct 27, 2023
20 of 22 checks passed

Saviq deleted the allow-llvmpipe-rendering branch October 27, 2023 05:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

platforms/renderer-generic-egl: Support CPUAddressableDisplayProvider #3095

platforms/renderer-generic-egl: Support CPUAddressableDisplayProvider #3095

RAOF commented Oct 25, 2023

RAOF commented Oct 25, 2023

RAOF commented Oct 25, 2023

mattkae commented Oct 25, 2023 •

edited

Loading

codecov bot commented Oct 25, 2023 •

edited

Loading

RAOF commented Oct 26, 2023

AlanGriffiths left a comment

Saviq commented Oct 26, 2023

AlanGriffiths commented Oct 26, 2023

Saviq left a comment

AlanGriffiths commented Oct 26, 2023

This comment was marked as off-topic.

This comment was marked as off-topic.

platforms/renderer-generic-egl: Support CPUAddressableDisplayProvider #3095

platforms/renderer-generic-egl: Support CPUAddressableDisplayProvider #3095

Conversation

RAOF commented Oct 25, 2023

RAOF commented Oct 25, 2023

RAOF commented Oct 25, 2023

mattkae commented Oct 25, 2023 • edited Loading

codecov bot commented Oct 25, 2023 • edited Loading

Codecov Report

RAOF commented Oct 26, 2023

AlanGriffiths left a comment

Choose a reason for hiding this comment

Saviq commented Oct 26, 2023

AlanGriffiths commented Oct 26, 2023

Saviq left a comment

Choose a reason for hiding this comment

AlanGriffiths commented Oct 26, 2023

This comment was marked as off-topic.

This comment was marked as off-topic.

mattkae commented Oct 25, 2023 •

edited

Loading

codecov bot commented Oct 25, 2023 •

edited

Loading