Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opengl: force using OpenGLES instead of OpenGL #33

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

opengl: force using OpenGLES instead of OpenGL #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a337a95
build: ensure that changes to helpers.cpp is picked up
ahayzen-kdab Mar 18, 2024
6fb633d
opengl: force using OpenGLES instead of OpenGL
ahayzen-kdab Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,4 @@ cxx-qt-lib-headers = { git = "https://github.com/kdab/cxx-qt.git", branch = "mai
qt-build-utils = { git = "https://github.com/kdab/cxx-qt.git", branch = "main" }

# We need a patched surfman to avoid OpenGL errors
surfman = { git = "https://github.com/ahayzen-kdab/surfman.git", branch = "patched-opengl-assert-0-9-1" }
surfman = { git = "https://github.com/ahayzen-kdab/surfman.git", branch = "patched-opengl-assert-and-gles-0-9-1" }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we only need the surfman side of this, Qt picks GLES correctly when available.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the embedded world branch has this change and worked well :-) https://github.com/KDABLabs/cxx-qt-servo-webview/tree/demo-embedded-world

0c76d94

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep :)
So we can close this PR, as forcing GLES on desktop is probably not wanted?

Otherwise it would be best to make upstream aware of this problem (or more likely fix it directly ourself).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we create an issue on this repo for now? We had to hardcode surfman having it detect or have an option somewhere to pick GLES over GL on Linux is what we would need.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So looking at the patch, I actually don't think it would need much to fix this upstream.

I suppose instead of hardcoding a return value, instead look at the underlying EGL connection (for Wayland at least) and return the OpenGL version from there?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right it somehow needs to determine from the connection or however Qt is doing it ? And potentially on X11 too ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right it somehow needs to determine from the connection

Yes, but that's pretty straight forward with EGL, there is eglQueryContext with the attribute EGL_CONTEXT_CLIENT_TYPE, which can return one of:

  • EGL_OPENGL_API
  • EGL_OPENGL_ES_API
  • EGL_OPENVG_API

I suppose the following patch should work (for Wayland platform):

diff --git a/surfman/src/platform/unix/wayland/connection.rs b/surfman/src/platform/unix/wayland/connection.rs
index 5fdcbbf..6161de6 100644
--- a/surfman/src/platform/unix/wayland/connection.rs
+++ b/surfman/src/platform/unix/wayland/connection.rs
@@ -63,7 +63,27 @@ impl Connection {
     /// Returns the OpenGL API flavor that this connection supports (OpenGL or OpenGL ES).
     #[inline]
     pub fn gl_api(&self) -> GLApi {
-        GLApi::GL
+        unsafe {
+            EGL_FUNCTIONS.with(|egl| {
+                let display = egl.GetCurrentDisplay();
+                let context = egl.GetCurrentContext();
+                if context != egl::NO_CONTEXT {
+                    let mut client_type: i32 = 0;
+                    egl.QueryContext(
+                        display,
+                        context,
+                        egl::CONTEXT_CLIENT_TYPE as i32,
+                        &mut client_type,
+                    );
+                    match client_type as u32 {
+                        egl::OPENGL_ES_API => GLApi::GLES,
+                        _ => GLApi::GL,
+                    }
+                } else {
+                    GLApi::GL
+                }
+            })
+        }
     }
 
     /// Returns the "best" adapter on this system, preferring high-performance hardware adapters.

I can't test it right now unfortunately, as the Pi is not online anymore.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice! Maybe ask @LeonMatthesKDAB when he is around to leave the device on for a day, as then we could submit this upstream :-)

3 changes: 2 additions & 1 deletion build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ fn main() {
.qt_module("OpenGL")
.qml_module(QmlModule {
uri: "com.kdab.servo",
rust_files: &["src/webview.rs"],
rust_files: &["src/webview.rs", "src/main.rs"],
qml_files: &["qml/main.qml", "qml/ServoToolbar.qml"],
qrc_files: &[
"images/arrow-back.png",
Expand All @@ -24,6 +24,7 @@ fn main() {
.cc_builder(|cc| {
cc.include("cpp");
cc.file("cpp/helpers.cpp");
println!("cargo:rerun-if-changed=cpp/helpers.cpp");
})
.file("src/renderer.rs")
.qobject_header("cpp/helpers.h")
Expand Down
9 changes: 9 additions & 0 deletions cpp/helpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <QOpenGLFramebufferObject>
#include <QOpenGLContext>
#include <QOpenGLFunctions>
#include <QSurfaceFormat>

void
blitFramebuffer(QOpenGLFramebufferObject* target, ::std::unique_ptr<QOpenGLFramebufferObject> source)
Expand Down Expand Up @@ -41,3 +42,11 @@ qTouchEventPoint(QTouchEvent& event, ::rust::isize index)
{
return (event.point(static_cast<qsizetype>(index)));
}

void forceSurfaceFormat()
{
QSurfaceFormat fmt;
fmt.setVersion(3, 0);
fmt.setRenderableType(QSurfaceFormat::OpenGLES);
QSurfaceFormat::setDefaultFormat(fmt);
}
2 changes: 2 additions & 0 deletions cpp/helpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,5 @@ QEventPoint const&
qTouchEventPoint(QTouchEvent& event, ::rust::isize index);

using QMouseEventButton = Qt::MouseButton;

void forceSurfaceFormat();
14 changes: 14 additions & 0 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,25 @@ mod servothread;
mod webview;
mod windowheadless;

#[cxx::bridge]
mod ffi {
unsafe extern "C++" {
include!("helpers.h");

#[cxx_name = "forceSurfaceFormat"]
fn force_surface_format();
}
}

fn main() {
// We need the OpenGL backend for QQuickFramebufferObject
std::env::set_var("QSG_RHI_BACKEND", "opengl");

let mut app = QGuiApplication::new();

// Force that we want OpenGLES
ffi::force_surface_format();

let mut engine = QQmlApplicationEngine::new();

if let Some(engine) = engine.as_mut() {
Expand Down