Skip to content

perf: SG-43585: Performance improvement of pyevaluate and pyexec #1375

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

Open
bernie-laberge wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf: SG-43585: Performance improvement of pyevaluate and pyexec #1375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions cmake/dependencies/crashpad.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ SET(_crashpad_handler
LIST(APPEND _configure_options "-DCMAKE_POSITION_INDEPENDENT_CODE=ON")
LIST(APPEND _configure_options "-DCRASHPAD_BUILD_TESTS=OFF")

# Pin the MSVC toolset to match the parent build. Without this, cmake auto-selects the newest installed toolset (e.g. 14.44) while the main build uses a pinned
# version (e.g. 14.40). The mismatch causes LNK2019 for __std_find_end_1 and similar CRT symbols added only in newer toolsets. Only applies to Visual Studio
# generators; the crashpad build dir must be clean.
IF(CMAKE_GENERATOR MATCHES "Visual Studio"
AND CMAKE_GENERATOR_TOOLSET
)
LIST(APPEND _configure_options "-T")
LIST(APPEND _configure_options "${CMAKE_GENERATOR_TOOLSET}")
ENDIF()

# Disable deprecation warnings on macOS — mini_chromium uses deprecated Security APIs
IF(RV_TARGET_DARWIN)
LIST(APPEND _configure_options "-DCMAKE_CXX_FLAGS=-Wno-error=deprecated-declarations -Wno-deprecated-declarations")
Expand Down
4 changes: 3 additions & 1 deletion docs/rv-manuals/rv-reference-manual/rv-reference-manual-chapter-four.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,9 @@ import rv.qtutils
# Gets the current RV session windows as a PySide QMainWindow.
rvSessionWindow = rv.qtutils.sessionWindow()

# Gets the current RV session GL view as a PySide QGLWidget.
# Gets the current RV session view host as a PySide QWidget. Note: the GL
# viewport is now a native child window embedded in this host (via
# createWindowContainer), so this is a plain QWidget, not a QOpenGLWidget.
rvSessionGLView = rv.qtutils.sessionGLView()

# Gets the current RV session top tool bar as a PySide QToolBar.
Expand Down
13 changes: 8 additions & 5 deletions src/bin/apps/rv/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,11 +353,6 @@ int utf8Main(int argc, char* argv[])

TwkFB::ThreadPool::initialize();

// Qt 5.12.1 specific
// Disable Qt Quick hardware rendering because QwebEngineView conflicts with
// QGLWidget
setEnvVar("QT_QUICK_BACKEND", "software");

#if defined(PLATFORM_LINUX)
// Work around for Wacom Tablet issue on linux
// Note: This is a Qt 5.12.4 regression
Expand All @@ -369,6 +364,14 @@ int utf8Main(int argc, char* argv[])
// (RV Preferences/Rendering/Multithread GPU Upload)
QApplication::setAttribute(Qt::AA_DontCheckOpenGLContextThreadAffinity);

// Share GL resources across every QOpenGLContext in the process. This is a
// documented QtWebEngine requirement, and it lets RV's auxiliary GL
// surfaces -- the second-output ScreenView and the multithreaded-upload
// worker device -- share textures/FBOs with the main viewport context
// without an explicit, ordering-sensitive setShareContext() call. Must be
// set before the QApplication is constructed.
QApplication::setAttribute(Qt::AA_ShareOpenGLContexts);

TwkUtil::MemPool::initialize();

string altPrefsPath;
Expand Down
31 changes: 28 additions & 3 deletions src/bin/nsapps/RV/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,34 @@ int main(int argc, char* argv[])

setPlatformSpecificLocale();

// Qt 5.12.1 specific
// Disable Qt Quick hardware rendering because QwebEngineView conflicts with
// QGLWidget
//
// Keep Qt Quick on the software backend. Do not remove this on macOS.
//
// It was originally added because QWebEngineView conflicted with the
// QGLWidget viewport, and src/bin/apps/rv/main.cpp has since dropped it for
// Windows and Linux -- but on macOS it is now load bearing for a different
// reason. The viewport is a native QOpenGLWindow embedded via
// QWidget::createWindowContainer, which makes it a QObject child of the top
// level window's QWidgetWindow. With the hardware Qt Quick backend, adding a
// QWebEngineView to the widget tree makes Qt destroy and recreate that
// native subtree; the viewport window is deleted with it rather than
// reparented, and QWindowContainer then dereferences the window it just
// lost. RV segfaults inside Qt, before it can react.
//
// Reproducible in four lines from any docked panel -- with the software
// backend this survives, with the hardware backend it crashes:
//
// w = rv.qtutils.sessionWindow()
// d = QtWidgets.QDockWidget('t', w)
// v = QtWebEngineWidgets.QWebEngineView(d)
// d.setWidget(v); w.addDockWidget(QtCore.Qt.RightDockWidgetArea, d)
// v.setHtml('<b>hi</b>')
//
// The cost of keeping it is that web panels composite in software on macOS,
// so macOS gets the cheap-viewport-repaint half of SG-43585 but not the
// hardware web panel half. Removing it needs the viewport to stop being
// owned by the top level window's native subtree, not just a comment change.
//
setenv("QT_QUICK_BACKEND", "software", 0 /* changeFlag : Do not change the existing value */);

// Prevent usage of native sibling widgets on Mac. This attribute can be
Expand Down
1 change: 1 addition & 0 deletions src/lib/app/RvCommon/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ SET(_sources
RvApplication.cpp
DiagnosticsView.cpp
GLView.cpp
GLWindow.cpp
TwkQTAction.cpp
MediaDirModel.cpp
RvNetworkDialog.cpp
Expand Down
12 changes: 6 additions & 6 deletions src/lib/app/RvCommon/DesktopVideoDevice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,10 @@ namespace Rv
{
TWK_GLDEBUG;

QSurfaceFormat fmt = shareDevice()->widget()->format();
QSurfaceFormat fmt = shareDevice()->glSurfaceFormat();
fmt.setSwapInterval(m_vsync ? 1 : 0);

ScreenView* vw = new ScreenView(fmt, 0, shareDevice()->widget(), Qt::Window);
ScreenView* vw = new ScreenView(fmt, 0, shareDevice()->glShareContext(), Qt::Window);
setViewWidget(vw);

QTGLVideoDevice* vd = new QTGLVideoDevice(0, "local view", vw);
Expand Down Expand Up @@ -747,11 +747,11 @@ namespace Rv

void DesktopVideoDevice::sortVideoFormatsByWidth() { sort(m_videoFormats.begin(), m_videoFormats.end(), widthSort); }

DesktopVideoDevice::ScreenView::ScreenView(const QSurfaceFormat& fmt, QWidget* parent, QOpenGLWidget* glViewShare,
DesktopVideoDevice::ScreenView::ScreenView(const QSurfaceFormat& fmt, QWidget* parent, QOpenGLContext* glShareContext,
Qt::WindowFlags flags)
: QOpenGLWidget(parent, flags)
{
m_glViewShare = glViewShare;
m_glShareContext = glShareContext;
setFormat(fmt);

// Important: set PartialUpdate, because otherwise
Expand All @@ -764,9 +764,9 @@ namespace Rv
{
QOpenGLWidget::initializeGL();

if (m_glViewShare && context() && context()->isValid())
if (m_glShareContext && context() && context()->isValid())
{
context()->setShareContext(m_glViewShare->context());
context()->setShareContext(m_glShareContext);
}
}

Expand Down
Loading
Loading