Skip to content

Commit

Permalink
Rewrite the threaded RenderWidget part loading
Browse files Browse the repository at this point in the history 
There are a lot of Sentry reports of crashes in this area, and they
all point towards the old updateGeometries() co-routine being called
again while it was awaiting the geometry calculations in another
thread.

The new version uses a more robust setup of 2 then-ables (loading and
geometry calculation), but both sync back to the main thread before
continuing. This also makes it easy to skip the geometry calculation,
if the user already selected a different part in the meantime.
  • Loading branch information
@rgriebl
rgriebl committed Oct 8, 2024
1 parent d95044b commit 9064f4f
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 148 deletions.
274 changes: 129 additions & 145 deletions src/ldraw/rendercontroller.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ RenderController::RenderController(QObject *parent)
, m_lineGeo(new QQuick3DGeometry())
, m_lines(new QmlRenderLineInstancing())
, m_clearColor(Qt::white)
, m_updateTimer(new QTimer(this))
{
static const float lineGeo[] = {
0, -0.5, 0,
Expand All @@ -46,17 +45,11 @@ RenderController::RenderController(QObject *parent)
m_lineGeo->setStride(3 * sizeof(float));
m_lineGeo->addAttribute(QQuick3DGeometry::Attribute::PositionSemantic, 0, QQuick3DGeometry::Attribute::F32Type);
m_lineGeo->setVertexData(QByteArray::fromRawData(reinterpret_cast<const char *>(lineGeo), sizeof(lineGeo)));

m_updateTimer->setInterval(200ms);
m_updateTimer->setSingleShot(true);
connect(m_updateTimer, &QTimer::timeout, this, &RenderController::updateGeometries);
m_updateTimer->start();
}

RenderController::~RenderController()
{
for (auto *geo : m_geos)
delete geo;
qDeleteAll(m_geos);
delete m_lines;
delete m_lineGeo;
if (m_part)
Expand Down Expand Up @@ -137,184 +130,175 @@ void RenderController::setItemAndColor(const BrickLink::Item *item, const BrickL
if (!color)
color = BrickLink::core()->color(0); // not-applicable

if (item == m_item) {
if (!item || (color == m_color))
return;

// switch color - redraw, if we have a Part loaded already
m_color = color;
if (m_part)
m_updateTimer->start();
} else {
// new item
if (m_part)
m_part->release();
m_part = nullptr;
m_item = item;
m_color = color;

m_updateTimer->start();

if (item) {
LDraw::library()->partFromBrickLinkId(item->id()).then(this, [this, item](Part *part) {
bool stillCurrentItem = (item == m_item);

if ((m_part != part) && stillCurrentItem) {
if (m_part)
m_part->release();
m_part = part;
if (m_part)
m_part->addRef();
}
// the future comes already ref'ed
if (part)
// no change
if ((item == m_item) && (!item || (color == m_color)))
return;

// new item
auto oldPart = m_part; // do not release immediately, as it might be re-used on color change
m_part = nullptr;
m_item = item;
m_color = color;

emit canRenderChanged(canRender());

if (item) {
LDraw::library()->partFromBrickLinkId(item->id())
.then(this, [this, item, color](Part *part) {
if (!part)
return;
if (item != m_item) {
part->release();
return;
}

if (m_part)
m_part->release();
m_part = part;
part->addRef();

emit canRenderChanged(canRender());

QtConcurrent::run(&RenderController::calculateRenderData, this, part, color)
.then(this, [this, color, part](RenderData data) {
part->release();
if (stillCurrentItem)
updateGeometries();
});
}
if (part != m_part)
return;
if (color != m_color) {
part->release();
return;
}
applyRenderData(data);
});
});
}
applyRenderData({});

if (oldPart)
oldPart->release();
}

bool RenderController::canRender() const
{
return (m_part);
}

QCoro::Task<void> RenderController::updateGeometries()
RenderController::RenderData RenderController::calculateRenderData(Part *part, const BrickLink::Color *color)
{
m_updateTimer->stop();

if (!m_part) {
qDeleteAll(m_geos);
m_geos.clear();
m_lines->clear();

emit surfacesChanged();
emit itemOrColorChanged();
emit canRenderChanged(canRender());
co_return;
}

// in
Part *part = m_part;
const BrickLink::Color *color = m_color;
if (!part)
return { };
part->addRef();

// out
float radius = 0;
QVector3D center;
QList<QmlRenderGeometry *> geos;
QByteArray lineBuffer;
QList<QmlRenderGeometry *> geos;
QVector3D center;
float radius = 0;
QHash<const BrickLink::Color *, QByteArray> surfaceBuffers;

QPointer<RenderController> guard(this);
fillVertexBuffers(part, color, color, QMatrix4x4(), false, surfaceBuffers, lineBuffer);

co_await QtConcurrent::run([part, color, &lineBuffer, &geos, &radius, &center]() {
QHash<const BrickLink::Color *, QByteArray> surfaceBuffers;
RenderController::fillVertexBuffers(part, color, color, QMatrix4x4(), false, surfaceBuffers, lineBuffer);
for (auto it = surfaceBuffers.cbegin(); it != surfaceBuffers.cend(); ++it) {
const QByteArray &data = it.value();
if (data.isEmpty())
continue;

for (auto it = surfaceBuffers.cbegin(); it != surfaceBuffers.cend(); ++it) {
const QByteArray &data = it.value();
if (data.isEmpty())
continue;
const BrickLink::Color *surfaceColor = it.key();
const bool isTextured = surfaceColor->hasParticles() || (surfaceColor->id() == 0);

const BrickLink::Color *surfaceColor = it.key();
const bool isTextured = surfaceColor->hasParticles() || (surfaceColor->id() == 0);
const int stride = (3 + 3 + (isTextured ? 2 : 0)) * sizeof(float);

const int stride = (3 + 3 + (isTextured ? 2 : 0)) * sizeof(float);
auto geo = new QmlRenderGeometry(surfaceColor);

auto geo = new QmlRenderGeometry(surfaceColor);
// calculate bounding box
static constexpr auto fmin = std::numeric_limits<float>::min();
static constexpr auto fmax = std::numeric_limits<float>::max();

// calculate bounding box
static constexpr auto fmin = std::numeric_limits<float>::min();
static constexpr auto fmax = std::numeric_limits<float>::max();
QVector3D vmin = QVector3D(fmax, fmax, fmax);
QVector3D vmax = QVector3D(fmin, fmin, fmin);

QVector3D vmin = QVector3D(fmax, fmax, fmax);
QVector3D vmax = QVector3D(fmin, fmin, fmin);
for (int i = 0; i < data.size(); i += stride) {
auto v = reinterpret_cast<const float *>(it->constData() + i);
vmin = QVector3D(std::min(vmin.x(), v[0]), std::min(vmin.y(), v[1]), std::min(vmin.z(), v[2]));
vmax = QVector3D(std::max(vmax.x(), v[0]), std::max(vmax.y(), v[1]), std::max(vmax.z(), v[2]));
}

for (int i = 0; i < data.size(); i += stride) {
auto v = reinterpret_cast<const float *>(it->constData() + i);
vmin = QVector3D(std::min(vmin.x(), v[0]), std::min(vmin.y(), v[1]), std::min(vmin.z(), v[2]));
vmax = QVector3D(std::max(vmax.x(), v[0]), std::max(vmax.y(), v[1]), std::max(vmax.z(), v[2]));
}
// calculate bounding sphere
QVector3D surfaceCenter = (vmin + vmax) / 2;
float surfaceRadius = 0;

// calculate bounding sphere
QVector3D surfaceCenter = (vmin + vmax) / 2;
float surfaceRadius = 0;
for (int i = 0; i < data.size(); i += stride) {
auto v = reinterpret_cast<const float *>(it->constData() + i);
surfaceRadius = std::max(surfaceRadius, (surfaceCenter - QVector3D { v[0], v[1], v[2] }).lengthSquared());
}
surfaceRadius = std::sqrt(surfaceRadius);

geo->setPrimitiveType(QQuick3DGeometry::PrimitiveType::Triangles);
geo->setStride(stride);
geo->addAttribute(QQuick3DGeometry::Attribute::PositionSemantic, 0, QQuick3DGeometry::Attribute::F32Type);
geo->addAttribute(QQuick3DGeometry::Attribute::NormalSemantic, 3 * sizeof(float), QQuick3DGeometry::Attribute::F32Type);
if (isTextured) {
geo->addAttribute(QQuick3DGeometry::Attribute::TexCoord0Semantic, 6 * sizeof(float), QQuick3DGeometry::Attribute::F32Type);

QQuick3DTextureData *texData = generateMaterialTextureData(surfaceColor);
texData->setParentItem(geo); // 3D scene parent
texData->setParent(geo); // owning parent
geo->setTextureData(texData);
}
geo->setBounds(vmin, vmax);
geo->setCenter(surfaceCenter);
geo->setRadius(surfaceRadius);
geo->setVertexData(data);

for (int i = 0; i < data.size(); i += stride) {
auto v = reinterpret_cast<const float *>(it->constData() + i);
surfaceRadius = std::max(surfaceRadius, (surfaceCenter - QVector3D { v[0], v[1], v[2] }).lengthSquared());
}
surfaceRadius = std::sqrt(surfaceRadius);

geo->setPrimitiveType(QQuick3DGeometry::PrimitiveType::Triangles);
geo->setStride(stride);
geo->addAttribute(QQuick3DGeometry::Attribute::PositionSemantic, 0, QQuick3DGeometry::Attribute::F32Type);
geo->addAttribute(QQuick3DGeometry::Attribute::NormalSemantic, 3 * sizeof(float), QQuick3DGeometry::Attribute::F32Type);
if (isTextured) {
geo->addAttribute(QQuick3DGeometry::Attribute::TexCoord0Semantic, 6 * sizeof(float), QQuick3DGeometry::Attribute::F32Type);

QQuick3DTextureData *texData = generateMaterialTextureData(surfaceColor);
texData->setParentItem(geo); // 3D scene parent
texData->setParent(geo); // owning parent
geo->setTextureData(texData);
}
geo->setBounds(vmin, vmax);
geo->setCenter(surfaceCenter);
geo->setRadius(surfaceRadius);
geo->setVertexData(data);
geos.append(geo);
}

geos.append(geo);
}
for (auto *geo : std::as_const(geos)) {
// Merge all the bounding spheres. This is not perfect, but very, very close in most cases
const auto geoCenter = geo->center();
const auto geoRadius = geo->radius();

for (auto *geo : std::as_const(geos)) {
// Merge all the bounding spheres. This is not perfect, but very, very close in most cases
const auto geoCenter = geo->center();
const auto geoRadius = geo->radius();
if (qFuzzyIsNull(radius)) { // first one
center = geoCenter;
radius = geoRadius;
} else {
QVector3D d = geoCenter - center;
float l = d.length();

if (qFuzzyIsNull(radius)) { // first one
if ((l + radius) < geoRadius) { // the old one is inside the new one
center = geoCenter;
radius = geoRadius;
} else {
QVector3D d = geoCenter - center;
float l = d.length();

if ((l + radius) < geoRadius) { // the old one is inside the new one
center = geoCenter;
radius = geoRadius;
} else if ((l + geoRadius) > radius) { // the new one is NOT inside the old one -> we need to merge
float nr = (radius + l + geoRadius) / 2;
center = center + (geoCenter - center).normalized() * (nr - radius);
radius = nr;
}
} else if ((l + geoRadius) > radius) { // the new one is NOT inside the old one -> we need to merge
float nr = (radius + l + geoRadius) / 2;
center = center + (geoCenter - center).normalized() * (nr - radius);
radius = nr;
}
}
});
}

if (!guard) // Sentry report: we've been deleted while the co-routine was running
co_return;
part->release();
return { lineBuffer, geos, center, radius };
}

qDeleteAll(m_geos);
m_geos.clear();
void RenderController::applyRenderData(const RenderData &data)
{
m_lines->clear();

m_geos = geos;
emit surfacesChanged();

m_lines->setBuffer(lineBuffer);
if (!data.lineBuffer.isEmpty())
m_lines->setBuffer(data.lineBuffer);
m_lines->update();
qDeleteAll(m_geos);
m_geos = data.geos;

emit surfacesChanged();

if (m_center != center) {
m_center = center;
if (m_center != data.center) {
m_center = data.center;
emit centerChanged();
}
if (!qFuzzyCompare(m_radius, radius)) {
m_radius = radius;
if (!qFuzzyCompare(m_radius, data.radius)) {
m_radius = data.radius;
emit radiusChanged();
}
emit itemOrColorChanged();
emit canRenderChanged(canRender());
}

std::vector<std::pair<float, float>> RenderController::uvMapToNearestPlane(const QVector3D &normal,
Expand Down
13 changes: 10 additions & 3 deletions src/ldraw/rendercontroller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,16 @@ public slots:
void clearColorChanged(const QColor &clearColor);

private:
QCoro::Task<void> updateGeometries();
struct RenderData {
QByteArray lineBuffer;
QList<QmlRenderGeometry *> geos;
QVector3D center;
float radius = 0;
};

RenderData calculateRenderData(Part *part, const BrickLink::Color *color);
void applyRenderData(const RenderData &data);

static void fillVertexBuffers(Part *part, const BrickLink::Color *modelColor,
const BrickLink::Color *baseColor, const QMatrix4x4 &matrix,
bool inverted, QHash<const BrickLink::Color *, QByteArray> &surfaceBuffers,
Expand All @@ -107,8 +116,6 @@ public slots:
float m_radius = 0;
bool m_tumblingAnimationActive = false;
QColor m_clearColor;

QTimer *m_updateTimer;
};

#if QT_VERSION >= QT_VERSION_CHECK(6, 5, 0)
Expand Down

0 comments on commit 9064f4f

Please sign in to comment.