Skip to content

Commit

Permalink
platforms/atomic-kms: Fix cursors across multiple monitors. (#3742)
Browse files Browse the repository at this point in the history
`KMSOutput::has_cursor` was misleadingly named, and so we implemented it
incorrectly. It is *used* to tell whether or not a cursor image needs to
be set on that particular output before it is visible, *not* whether
that output *supports* a cursor.

Rename it to something hopefully more easily understandable, and then
fix the implementation.

Fixes: #3677
  • Loading branch information
AlanGriffiths authored Feb 10, 2025
2 parents 2e7ec78 + 917387a commit 473cc62
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 18 deletions.
16 changes: 6 additions & 10 deletions src/platforms/atomic-kms/server/kms/atomic_kms_output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ bool mga::AtomicKMSOutput::page_flip(FBHandle const& fb)
return true;
}

void mga::AtomicKMSOutput::set_cursor(gbm_bo* buffer)
void mga::AtomicKMSOutput::set_cursor_image(gbm_bo* buffer)
{
if (auto conf = configuration.lock(); conf->current_crtc)
{
Expand All @@ -490,6 +490,7 @@ void mga::AtomicKMSOutput::set_cursor(gbm_bo* buffer)
{
mir::log_warning("set_cursor: drmModeSetCursor failed (%s)", strerror(-result));
}
cursor_image_set = true;
}
}

Expand All @@ -516,6 +517,8 @@ bool mga::AtomicKMSOutput::clear_cursor()
mir::log_warning("clear_cursor: drmModeSetCursor failed (%s)", strerror(-result));
}

cursor_image_set = false;

return !result;
}

Expand Down Expand Up @@ -920,15 +923,8 @@ int mga::AtomicKMSOutput::drm_fd() const
return drm_fd_;
}

bool mga::AtomicKMSOutput::has_cursor() const
bool mga::AtomicKMSOutput::has_cursor_image() const
{
// According to this:
// https://github.com/canonical/mir/pull/3665#discussion_r1835985441. The
// only point of failure is in the `Cursor` constructor, which throws an
// exception, which is caught in `mga::create_hardware_cursor` and signals
// to the calling code to use a software cursor instead.
//
// tldr; If this method is called, we have a cursor.
return true;
return cursor_image_set;
}

5 changes: 3 additions & 2 deletions src/platforms/atomic-kms/server/kms/atomic_kms_output.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ class AtomicKMSOutput : public KMSOutput
void clear_crtc() override;
bool page_flip(FBHandle const& fb) override;

void set_cursor(gbm_bo* buffer) override;
void set_cursor_image(gbm_bo* buffer) override;
void move_cursor(geometry::Point destination) override;
bool clear_cursor() override;
bool has_cursor() const override;
bool has_cursor_image() const override;

void set_power_mode(MirPowerMode mode) override;
void set_gamma(GammaCurves const& gamma) override;
Expand Down Expand Up @@ -93,6 +93,7 @@ class AtomicKMSOutput : public KMSOutput
mir::Synchronised<Configuration> configuration;
drmModeCrtc saved_crtc;
bool using_saved_crtc;
std::atomic<bool> cursor_image_set{false};
};

}
Expand Down
8 changes: 4 additions & 4 deletions src/platforms/atomic-kms/server/kms/cursor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -391,16 +391,16 @@ void mga::Cursor::place_cursor_at_locked(
if (changed_orientation)
pad_and_write_image_data_locked(lg, buffer);

if (force_state || !output.has_cursor() || changed_orientation)
if (force_state || !output.has_cursor_image() || changed_orientation)
{
output.set_cursor(buffer);
if (!output.has_cursor())
output.set_cursor_image(buffer);
if (!output.has_cursor_image())
set_on_all_outputs = false;
}
}
else
{
if (force_state || output.has_cursor())
if (force_state || output.has_cursor_image())
{
output.clear_cursor();
}
Expand Down
4 changes: 2 additions & 2 deletions src/platforms/atomic-kms/server/kms/kms_output.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,10 @@ class KMSOutput

virtual bool page_flip(FBHandle const& fb) = 0;

virtual void set_cursor(gbm_bo* buffer) = 0;
virtual void set_cursor_image(gbm_bo* buffer) = 0;
virtual void move_cursor(geometry::Point destination) = 0;
virtual bool clear_cursor() = 0;
virtual bool has_cursor() const = 0;
virtual bool has_cursor_image() const = 0;

virtual void set_power_mode(MirPowerMode mode) = 0;
virtual void set_gamma(GammaCurves const& gamma) = 0;
Expand Down

0 comments on commit 473cc62

Please sign in to comment.