fix: remaining issues recorded during test coverage work
- RenderManager: GPU-side members (context_, decoder_cache_, shader_cache_, auto_cacher_, worker_pool_, decoder_clear_timer_) were left uninitialized when the configured graphics backend is unknown (e.g. dummy); ViewerWidget then dereferenced garbage and crashed. Initialize them at declaration - CurveView::SelectKeyframesOfInput ignored its reference parameter and selected keyframes of every connected track; select only the requested track's keyframes - SeekableWidget::SeekToScenePoint dereferenced GetViewerNode() unconditionally; skip the playhead update when no viewer is connected - LoadOTIOTask: unknown root schema leaked the freshly allocated project_ (delete + reset; OTIO is not enabled in local builds so this file is compile-verified by inspection only) Locked by new tests: RenderManagerDummyBackend, TimeRuler.SeekToScenePointWithoutViewerIsNoOp, CurveViewTest.SelectKeyframesOfInputSelectsOnlyRequestedTrack
This commit is contained in:
+10
-10
@@ -63,11 +63,11 @@ private:
|
|||||||
|
|
||||||
bool cancelled_;
|
bool cancelled_;
|
||||||
|
|
||||||
Renderer *context_;
|
Renderer *context_ = nullptr;
|
||||||
|
|
||||||
DecoderCache *decoder_cache_;
|
DecoderCache *decoder_cache_ = nullptr;
|
||||||
|
|
||||||
ShaderCache *shader_cache_;
|
ShaderCache *shader_cache_ = nullptr;
|
||||||
};
|
};
|
||||||
|
|
||||||
class RenderWorkerPool;
|
class RenderWorkerPool;
|
||||||
@@ -239,21 +239,21 @@ private:
|
|||||||
|
|
||||||
static RenderManager *instance_;
|
static RenderManager *instance_;
|
||||||
|
|
||||||
Renderer *context_;
|
Renderer *context_ = nullptr;
|
||||||
|
|
||||||
Backend backend_;
|
Backend backend_;
|
||||||
Backend requested_backend_;
|
Backend requested_backend_;
|
||||||
|
|
||||||
DecoderCache *decoder_cache_;
|
DecoderCache *decoder_cache_ = nullptr;
|
||||||
|
|
||||||
ShaderCache *shader_cache_;
|
ShaderCache *shader_cache_ = nullptr;
|
||||||
|
|
||||||
static constexpr auto kDecoderMaximumInactivityAggressive = 1000;
|
static constexpr auto kDecoderMaximumInactivityAggressive = 1000;
|
||||||
static constexpr auto kDecoderMaximumInactivity = 5000;
|
static constexpr auto kDecoderMaximumInactivity = 5000;
|
||||||
|
|
||||||
int aggressive_gc_;
|
int aggressive_gc_ = 0;
|
||||||
|
|
||||||
QTimer *decoder_clear_timer_;
|
QTimer *decoder_clear_timer_ = nullptr;
|
||||||
|
|
||||||
RenderThread *dry_run_thread_ = nullptr;
|
RenderThread *dry_run_thread_ = nullptr;
|
||||||
RenderThread *audio_thread_ = nullptr;
|
RenderThread *audio_thread_ = nullptr;
|
||||||
@@ -263,9 +263,9 @@ private:
|
|||||||
|
|
||||||
std::list<RenderThread *> render_threads_;
|
std::list<RenderThread *> render_threads_;
|
||||||
|
|
||||||
PreviewAutoCacher *auto_cacher_;
|
PreviewAutoCacher *auto_cacher_ = nullptr;
|
||||||
|
|
||||||
RenderWorkerPool *worker_pool_;
|
RenderWorkerPool *worker_pool_ = nullptr;
|
||||||
|
|
||||||
private slots:
|
private slots:
|
||||||
void ClearOldDecoders();
|
void ClearOldDecoders();
|
||||||
|
|||||||
@@ -94,6 +94,8 @@ bool LoadOTIOTask::Run()
|
|||||||
} else {
|
} else {
|
||||||
// Unknown root, we don't know what to do with this
|
// Unknown root, we don't know what to do with this
|
||||||
SetError(tr("Unknown OpenTimelineIO root element"));
|
SetError(tr("Unknown OpenTimelineIO root element"));
|
||||||
|
delete project_;
|
||||||
|
project_ = nullptr;
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -91,7 +91,7 @@ void CurveView::SelectKeyframesOfInput(const NodeKeyframeTrackReference &ref)
|
|||||||
{
|
{
|
||||||
DeselectAll();
|
DeselectAll();
|
||||||
|
|
||||||
foreach (KeyframeViewInputConnection *con, track_connections_) {
|
if (KeyframeViewInputConnection *con = track_connections_.value(ref)) {
|
||||||
foreach (NodeKeyframe *key, con->GetKeyframes()) {
|
foreach (NodeKeyframe *key, con->GetKeyframes()) {
|
||||||
SelectKeyframe(key);
|
SelectKeyframe(key);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -433,8 +433,9 @@ void SeekableWidget::SeekToScenePoint(qreal scene)
|
|||||||
playhead_time += movement;
|
playhead_time += movement;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (playhead_time != GetViewerNode()->GetPlayhead()) {
|
ViewerOutput *viewer = GetViewerNode();
|
||||||
GetViewerNode()->SetPlayhead(playhead_time);
|
if (viewer && playhead_time != viewer->GetPlayhead()) {
|
||||||
|
viewer->SetPlayhead(playhead_time);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -712,3 +712,22 @@ TEST_F(RenderMiscAutoCacherTest, CacheProxyTaskCancelledClearsPendingJobs)
|
|||||||
|
|
||||||
cacher.SetProject(nullptr);
|
cacher.SetProject(nullptr);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// With an unknown/dummy graphics backend the RenderManager never creates the
|
||||||
|
// GPU-side objects. Those pointers must be null (previously they were left
|
||||||
|
// uninitialized, so callers such as ViewerWidget dereferenced garbage and
|
||||||
|
// crashed).
|
||||||
|
TEST(RenderManagerDummyBackend, GpuMembersAreNullRatherThanUninitialized)
|
||||||
|
{
|
||||||
|
const QVariant previous =
|
||||||
|
olive::Config::Current()[QStringLiteral("GraphicsBackend")];
|
||||||
|
olive::Config::Current()[QStringLiteral("GraphicsBackend")] =
|
||||||
|
QStringLiteral("dummy");
|
||||||
|
|
||||||
|
olive::RenderManager::CreateInstance();
|
||||||
|
|
||||||
|
EXPECT_EQ(olive::RenderManager::instance()->GetCacher(), nullptr);
|
||||||
|
|
||||||
|
olive::RenderManager::DestroyInstance();
|
||||||
|
olive::Config::Current()[QStringLiteral("GraphicsBackend")] = previous;
|
||||||
|
}
|
||||||
|
|||||||
@@ -344,6 +344,34 @@ TEST_F(CurveViewTest, SetKeyframeTrackColorAppliesToBrush)
|
|||||||
EXPECT_EQ(connection->GetBrush().color(), QColor(Qt::blue));
|
EXPECT_EQ(connection->GetBrush().color(), QColor(Qt::blue));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
TEST_F(CurveViewTest, SelectKeyframesOfInputSelectsOnlyRequestedTrack)
|
||||||
|
{
|
||||||
|
class SelectionProbeCurveView : public CurveView {
|
||||||
|
public:
|
||||||
|
using KeyframeView::IsKeyframeSelected;
|
||||||
|
};
|
||||||
|
|
||||||
|
SelectionProbeCurveView view;
|
||||||
|
view.ConnectInput(ColorTrackRef(0));
|
||||||
|
view.ConnectInput(ColorTrackRef(1));
|
||||||
|
|
||||||
|
NodeKeyframe *key0 =
|
||||||
|
InsertKeyframe(solid_, SolidGenerator::kColorInput, rational(0), 0.5, 0);
|
||||||
|
NodeKeyframe *key1 =
|
||||||
|
InsertKeyframe(solid_, SolidGenerator::kColorInput, rational(1), 0.6, 1);
|
||||||
|
|
||||||
|
// Previously the reference was ignored and keyframes of every connected
|
||||||
|
// track got selected.
|
||||||
|
view.SelectKeyframesOfInput(ColorTrackRef(0));
|
||||||
|
EXPECT_TRUE(view.IsKeyframeSelected(key0));
|
||||||
|
EXPECT_FALSE(view.IsKeyframeSelected(key1));
|
||||||
|
|
||||||
|
// Selecting the other track replaces the selection (DeselectAll first)
|
||||||
|
view.SelectKeyframesOfInput(ColorTrackRef(1));
|
||||||
|
EXPECT_FALSE(view.IsKeyframeSelected(key0));
|
||||||
|
EXPECT_TRUE(view.IsKeyframeSelected(key1));
|
||||||
|
}
|
||||||
|
|
||||||
TEST(CurveWidget, VerticalScaleRoundTripsThroughView)
|
TEST(CurveWidget, VerticalScaleRoundTripsThroughView)
|
||||||
{
|
{
|
||||||
ColorManager::SetUpDefaultConfig();
|
ColorManager::SetUpDefaultConfig();
|
||||||
|
|||||||
@@ -100,6 +100,20 @@ TEST(TimeRuler, SeekToScenePointWithoutTimebaseIsNoOp)
|
|||||||
SUCCEED();
|
SUCCEED();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
TEST(TimeRuler, SeekToScenePointWithoutViewerIsNoOp)
|
||||||
|
{
|
||||||
|
EnsureAppSingletons();
|
||||||
|
|
||||||
|
// Timebase set but no viewer connected: previously dereferenced a null
|
||||||
|
// GetViewerNode() and crashed.
|
||||||
|
TimeRuler ruler;
|
||||||
|
ruler.SetTimebase(rational(1, 30));
|
||||||
|
ruler.SetScale(100.0);
|
||||||
|
|
||||||
|
ruler.SeekToScenePoint(150.0);
|
||||||
|
SUCCEED();
|
||||||
|
}
|
||||||
|
|
||||||
class PlaybackControlsTest : public ::testing::Test {
|
class PlaybackControlsTest : public ::testing::Test {
|
||||||
protected:
|
protected:
|
||||||
void SetUp() override
|
void SetUp() override
|
||||||
|
|||||||
Reference in New Issue
Block a user