Skip to content

Commit

Permalink
Qt: do not wrap QPainter directly, but use a unique_ptr instead
Browse files Browse the repository at this point in the history
Because QPainter can't be relocated.

Fixes #1230
  • Loading branch information
ogoffart committed May 5, 2022
1 parent 696ce93 commit 6893884
Show file tree
Hide file tree
Showing 13 changed files with 177 additions and 151 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
@@ -1,6 +1,16 @@
# Changelog
All notable changes to this project are documented in this file.

## Unreleased

### Changed

### Added

### Fixed

- Fixed crashes with the Qt backend in release mode.

## [0.2.2] - 2022-05-04

### Changed
Expand Down
19 changes: 11 additions & 8 deletions internal/backends/qt/qt_widgets.rs
Expand Up @@ -17,6 +17,7 @@ it needs to be kept in sync with different place.

#![allow(non_upper_case_globals)]

use crate::qt_window::QPainterPtr;
use const_field_offset::FieldOffsets;
use core::pin::Pin;
use cpp::cpp;
Expand All @@ -38,8 +39,6 @@ use std::rc::Rc;

type ItemRendererRef<'a> = &'a mut dyn ItemRenderer;

use qttypes::QPainter;

/// Helper macro to get the size from the width and height property,
/// and return Default::default in case the size is too small
macro_rules! get_size {
Expand Down Expand Up @@ -69,12 +68,12 @@ macro_rules! fn_render {
return (int)state;
});

if let Some(painter) = <dyn std::any::Any>::downcast_mut::<QPainter>(backend.as_any()) {
if let Some(painter) = <dyn std::any::Any>::downcast_mut::<QPainterPtr>(backend.as_any()) {
let $size: qttypes::QSize = get_size!(self);
let $this = self;
painter.save();
let $widget = cpp!(unsafe [painter as "QPainter*"] -> * const () as "QWidget*" {
return painter->device()->devType() == QInternal::Widget ? static_cast<QWidget *>(painter->device()) : nullptr;
let $widget = cpp!(unsafe [painter as "QPainterPtr*"] -> * const () as "QWidget*" {
return (*painter)->device()->devType() == QInternal::Widget ? static_cast<QWidget *>((*painter)->device()) : nullptr;
});
let $painter = painter;
$($tt)*
Expand All @@ -92,12 +91,14 @@ macro_rules! fn_render {
let $size = qttypes::QSize { width: width as _, height: height as _ };
let mut imgarray = QImageWrapArray::new($size, $dpr);
let img = &mut imgarray.img;
let mut painter_ = cpp!(unsafe [img as "QImage*"] -> QPainter as "QPainter" { return QPainter(img); });
let mut painter = cpp!(unsafe [img as "QImage*"] -> QPainterPtr as "std::unique_ptr<QPainter>" {
return std::make_unique<QPainter>(img);
});
let $widget: * const () = core::ptr::null();
let $painter = &mut painter_;
let $painter = &mut painter;
let $this = self;
$($tt)*
drop(painter_);
drop(painter);
imgarray.draw(callback);
},
);
Expand Down Expand Up @@ -144,6 +145,8 @@ cpp! {{
#include <QtCore/QDebug>
#include <QtCore/QScopeGuard>

using QPainterPtr = std::unique_ptr<QPainter>;

/// Make sure there is an instance of QApplication.
/// The `from_qt_backend` argument specifies if we know that we are running
/// the Qt backend, or if we are just drawing widgets
Expand Down
4 changes: 2 additions & 2 deletions internal/backends/qt/qt_widgets/button.rs
Expand Up @@ -298,7 +298,7 @@ impl Item for NativeButton {
let has_focus = this.has_focus();

cpp!(unsafe [
painter as "QPainter*",
painter as "QPainterPtr*",
widget as "QWidget*",
text as "QString",
icon as "QPixmap",
Expand Down Expand Up @@ -328,7 +328,7 @@ impl Item for NativeButton {
if (has_focus) {
option.state |= QStyle::State_HasFocus | QStyle::State_KeyboardFocusChange | QStyle::State_Item;
}
qApp->style()->drawControl(QStyle::CE_PushButton, &option, painter, widget);
qApp->style()->drawControl(QStyle::CE_PushButton, &option, painter->get(), widget);
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions internal/backends/qt/qt_widgets/checkbox.rs
Expand Up @@ -109,7 +109,7 @@ impl Item for NativeCheckBox {
let text: qttypes::QString = this.text().as_str().into();

cpp!(unsafe [
painter as "QPainter*",
painter as "QPainterPtr*",
widget as "QWidget*",
enabled as "bool",
text as "QString",
Expand All @@ -132,7 +132,7 @@ impl Item for NativeCheckBox {
if (has_focus) {
option.state |= QStyle::State_HasFocus | QStyle::State_KeyboardFocusChange | QStyle::State_Item;
}
qApp->style()->drawControl(QStyle::CE_CheckBox, &option, painter, widget);
qApp->style()->drawControl(QStyle::CE_CheckBox, &option, painter->get(), widget);
});
}
}
Expand Down
6 changes: 3 additions & 3 deletions internal/backends/qt/qt_widgets/combobox.rs
Expand Up @@ -103,7 +103,7 @@ impl Item for NativeComboBox {
this.current_value().as_str().into();
let enabled = this.enabled();
cpp!(unsafe [
painter as "QPainter*",
painter as "QPainterPtr*",
widget as "QWidget*",
text as "QString",
enabled as "bool",
Expand Down Expand Up @@ -132,8 +132,8 @@ impl Item for NativeComboBox {
// option.state |= QStyle::State_On;
}
option.subControls = QStyle::SC_All;
qApp->style()->drawComplexControl(QStyle::CC_ComboBox, &option, painter, widget);
qApp->style()->drawControl(QStyle::CE_ComboBoxLabel, &option, painter, widget);
qApp->style()->drawComplexControl(QStyle::CC_ComboBox, &option, painter->get(), widget);
qApp->style()->drawControl(QStyle::CE_ComboBoxLabel, &option, painter->get(), widget);
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions internal/backends/qt/qt_widgets/groupbox.rs
Expand Up @@ -162,7 +162,7 @@ impl Item for NativeGroupBox {
let enabled = this.enabled();

cpp!(unsafe [
painter as "QPainter*",
painter as "QPainterPtr*",
widget as "QWidget*",
text as "QString",
enabled as "bool",
Expand All @@ -187,7 +187,7 @@ impl Item for NativeGroupBox {
}
option.textColor = QColor(qApp->style()->styleHint(
QStyle::SH_GroupBox_TextLabelColor, &option));
qApp->style()->drawComplexControl(QStyle::CC_GroupBox, &option, painter, widget);
qApp->style()->drawComplexControl(QStyle::CC_GroupBox, &option, painter->get(), widget);
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions internal/backends/qt/qt_widgets/lineedit.rs
Expand Up @@ -115,7 +115,7 @@ impl Item for NativeLineEdit {
let enabled: bool = this.enabled();

cpp!(unsafe [
painter as "QPainter*",
painter as "QPainterPtr*",
widget as "QWidget*",
size as "QSize",
dpr as "float",
Expand All @@ -135,7 +135,7 @@ impl Item for NativeLineEdit {
} else {
option.palette.setCurrentColorGroup(QPalette::Disabled);
}
qApp->style()->drawPrimitive(QStyle::PE_PanelLineEdit, &option, painter, widget);
qApp->style()->drawPrimitive(QStyle::PE_PanelLineEdit, &option, painter->get(), widget);
});
}
}
Expand Down
10 changes: 5 additions & 5 deletions internal/backends/qt/qt_widgets/listviewitem.rs
Expand Up @@ -90,7 +90,7 @@ impl Item for NativeStandardListViewItem {
let item = this.item();
let text: qttypes::QString = item.text.as_str().into();
cpp!(unsafe [
painter as "QPainter*",
painter as "QPainterPtr*",
widget as "QWidget*",
size as "QSize",
dpr as "float",
Expand Down Expand Up @@ -120,14 +120,14 @@ impl Item for NativeStandardListViewItem {
option.features |= QStyleOptionViewItem::HasDisplay;
option.text = text;
// CE_ItemViewItem in QCommonStyle calls setClipRect on the painter and replace the clips. So we need to cheat.
auto engine = painter->paintEngine();
auto engine = (*painter)->paintEngine();
auto old_clip = engine->systemClip();
auto new_clip = old_clip & (painter->clipRegion() * painter->transform());
auto new_clip = old_clip & ((*painter)->clipRegion() * (*painter)->transform());
if (new_clip.isEmpty()) return;
engine->setSystemClip(new_clip);

qApp->style()->drawPrimitive(QStyle::PE_PanelItemViewRow, &option, painter, widget);
qApp->style()->drawControl(QStyle::CE_ItemViewItem, &option, painter, widget);
qApp->style()->drawPrimitive(QStyle::PE_PanelItemViewRow, &option, painter->get(), widget);
qApp->style()->drawControl(QStyle::CE_ItemViewItem, &option, painter->get(), widget);
engine->setSystemClip(old_clip);
});
}
Expand Down
25 changes: 13 additions & 12 deletions internal/backends/qt/qt_widgets/scrollview.rs
Expand Up @@ -272,7 +272,7 @@ impl Item for NativeScrollView {
let enabled: bool = this.enabled();
let has_focus: bool = this.has_focus();
let frame_around_contents = cpp!(unsafe [
painter as "QPainter*",
painter as "QPainterPtr*",
widget as "QWidget*",
size as "QSize",
dpr as "float",
Expand Down Expand Up @@ -303,13 +303,13 @@ impl Item for NativeScrollView {
QSize corner_size = QSize(margins.right() - margins.left(), margins.bottom() - margins.top());
if (foac) {
frameOption.rect = QRect(QPoint(), (size / dpr) - corner_size);
qApp->style()->drawControl(QStyle::CE_ShapedFrame, &frameOption, painter, widget);
qApp->style()->drawControl(QStyle::CE_ShapedFrame, &frameOption, painter->get(), widget);
frameOption.rect = QRect(frameOption.rect.bottomRight() + QPoint(1, 1), corner_size);
qApp->style()->drawPrimitive(QStyle::PE_PanelScrollAreaCorner, &frameOption, painter, widget);
qApp->style()->drawPrimitive(QStyle::PE_PanelScrollAreaCorner, &frameOption, painter->get(), widget);
} else {
qApp->style()->drawControl(QStyle::CE_ShapedFrame, &frameOption, painter, widget);
qApp->style()->drawControl(QStyle::CE_ShapedFrame, &frameOption, painter->get(), widget);
frameOption.rect = QRect(frameOption.rect.bottomRight() + QPoint(1, 1) - QPoint(margins.right(), margins.bottom()), corner_size);
qApp->style()->drawPrimitive(QStyle::PE_PanelScrollAreaCorner, &frameOption, painter, widget);
qApp->style()->drawPrimitive(QStyle::PE_PanelScrollAreaCorner, &frameOption, painter->get(), widget);
}
return foac;
});
Expand All @@ -323,7 +323,7 @@ impl Item for NativeScrollView {
pressed: bool,
initial_state: i32| {
cpp!(unsafe [
painter as "QPainter*",
painter as "QPainterPtr*",
widget as "QWidget*",
value as "int",
page_size as "int",
Expand All @@ -336,6 +336,7 @@ impl Item for NativeScrollView {
has_focus as "bool",
initial_state as "int"
] {
QPainter *painter_ = painter->get();
auto r = rect.toAlignedRect();
// The mac style may crash on invalid rectangles (#595)
if (!r.isValid())
Expand All @@ -345,11 +346,11 @@ impl Item for NativeScrollView {
#if defined(Q_OS_MAC)
QImage scrollbar_image(r.size(), QImage::Format_ARGB32_Premultiplied);
scrollbar_image.fill(Qt::transparent);
{QPainter p(&scrollbar_image); QPainter *painter = &p;
{QPainter p(&scrollbar_image); QPainter *painter_ = &p;
#else
painter->save();
auto cleanup = qScopeGuard([&] { painter->restore(); });
painter->translate(r.topLeft()); // There is bugs in the styles if the scrollbar is not in (0,0)
painter_->save();
auto cleanup = qScopeGuard([&] { painter_->restore(); });
painter_->translate(r.topLeft()); // There is bugs in the styles if the scrollbar is not in (0,0)
#endif
QStyleOptionSlider option;
option.state |= QStyle::State(initial_state);
Expand All @@ -366,10 +367,10 @@ impl Item for NativeScrollView {
}

auto style = qApp->style();
style->drawComplexControl(QStyle::CC_ScrollBar, &option, painter, widget);
style->drawComplexControl(QStyle::CC_ScrollBar, &option, painter_, widget);
#if defined(Q_OS_MAC)
}
painter->drawImage(r.topLeft(), scrollbar_image);
(painter_)->drawImage(r.topLeft(), scrollbar_image);
#endif
});
};
Expand Down
4 changes: 2 additions & 2 deletions internal/backends/qt/qt_widgets/slider.rs
Expand Up @@ -207,7 +207,7 @@ impl Item for NativeSlider {
let pressed = data.pressed;

cpp!(unsafe [
painter as "QPainter*",
painter as "QPainterPtr*",
widget as "QWidget*",
enabled as "bool",
value as "int",
Expand All @@ -224,7 +224,7 @@ impl Item for NativeSlider {
option.rect = QRect(QPoint(), size / dpr);
initQSliderOptions(option, pressed, enabled, active_controls, min, max, value);
auto style = qApp->style();
style->drawComplexControl(QStyle::CC_Slider, &option, painter, widget);
style->drawComplexControl(QStyle::CC_Slider, &option, painter->get(), widget);
});
}
}
Expand Down
10 changes: 5 additions & 5 deletions internal/backends/qt/qt_widgets/spinbox.rs
Expand Up @@ -231,7 +231,7 @@ impl Item for NativeSpinBox {
let pressed = data.pressed;

cpp!(unsafe [
painter as "QPainter*",
painter as "QPainterPtr*",
widget as "QWidget*",
value as "int",
enabled as "bool",
Expand All @@ -250,7 +250,7 @@ impl Item for NativeSpinBox {
}
option.rect = QRect(QPoint(), size / dpr);
initQSpinBoxOptions(option, pressed, enabled, active_controls);
style->drawComplexControl(QStyle::CC_SpinBox, &option, painter, widget);
style->drawComplexControl(QStyle::CC_SpinBox, &option, painter->get(), widget);

QStyleOptionFrame frame;
frame.state = option.state;
Expand All @@ -259,11 +259,11 @@ impl Item for NativeSpinBox {
: style->pixelMetric(QStyle::PM_DefaultFrameWidth, &option, widget);
frame.midLineWidth = 0;
frame.rect = style->subControlRect(QStyle::CC_SpinBox, &option, QStyle::SC_SpinBoxEditField, widget);
style->drawPrimitive(QStyle::PE_PanelLineEdit, &frame, painter, widget);
style->drawPrimitive(QStyle::PE_PanelLineEdit, &frame, painter->get(), widget);
QRect text_rect = qApp->style()->subElementRect(QStyle::SE_LineEditContents, &frame, widget);
text_rect.adjust(1, 2, 1, 2);
painter->setPen(option.palette.color(QPalette::Text));
painter->drawText(text_rect, QString::number(value));
(*painter)->setPen(option.palette.color(QPalette::Text));
(*painter)->drawText(text_rect, QString::number(value));
});
}
}
Expand Down
10 changes: 5 additions & 5 deletions internal/backends/qt/qt_widgets/tabwidget.rs
Expand Up @@ -252,7 +252,7 @@ impl Item for NativeTabWidget {
height: this.tabbar_preferred_height() as _,
};
cpp!(unsafe [
painter as "QPainter*",
painter as "QPainterPtr*",
widget as "QWidget*",
size as "QSize",
dpr as "float",
Expand All @@ -275,7 +275,7 @@ impl Item for NativeTabWidget {
option.leftCornerWidgetSize = QSize(0, 0);
option.tabBarRect = style->subElementRect(QStyle::SE_TabWidgetTabBar, &option, widget);
option.rect = style->subElementRect(QStyle::SE_TabWidgetTabPane, &option, widget);
style->drawPrimitive(QStyle::PE_FrameTabWidget, &option, painter, widget);
style->drawPrimitive(QStyle::PE_FrameTabWidget, &option, painter->get(), widget);

/* -- we don't need to draw the base since we already draw the frame
QStyleOptionTab tabOverlap;
Expand All @@ -290,7 +290,7 @@ impl Item for NativeTabWidget {
}
optTabBase.tabBarRect = option.tabBarRect;
optTabBase.selectedTabRect = option.selectedTabRect;
style->drawPrimitive(QStyle::PE_FrameTabBarBase, &optTabBase, painter, widget);*/
style->drawPrimitive(QStyle::PE_FrameTabBarBase, &optTabBase, painter->get(), widget);*/
});
}
}
Expand Down Expand Up @@ -449,7 +449,7 @@ impl Item for NativeTab {
let num_tabs: i32 = this.num_tabs();

cpp!(unsafe [
painter as "QPainter*",
painter as "QPainterPtr*",
widget as "QWidget*",
text as "QString",
icon as "QPixmap",
Expand Down Expand Up @@ -492,7 +492,7 @@ impl Item for NativeTab {
option.state |= QStyle::State_HasFocus | QStyle::State_KeyboardFocusChange | QStyle::State_Item;
}
option.features |= QStyleOptionTab::HasFrame;
qApp->style()->drawControl(QStyle::CE_TabBarTab, &option, painter, widget);
qApp->style()->drawControl(QStyle::CE_TabBarTab, &option, painter->get(), widget);
});
}
}
Expand Down

0 comments on commit 6893884

Please sign in to comment.