Skip to content

Fix scroll cursor ("arrow") position on screen edges for software rendered cursor #9852

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
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix scroll cursor ("arrow") position on screen edges for software rendered cursor #9852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fd20c5b
Fix scroll cursor position and update rendering
Districh-ru May 28, 2025
8fa6ca5
Update copyright
Districh-ru May 28, 2025
100f70d
Fix Clang-Tidy issues
Districh-ru May 28, 2025
a3f27d3
Merge branch 'ihhub:master' into fix-scroll-cursor-out-of-screen
Districh-ru May 28, 2025
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
69 changes: 38 additions & 31 deletions src/engine/screen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,40 +217,45 @@ namespace
return palette.data();
}

// Updates `roi` to be only inside {0, 0, width, height} rectangle.
// Returns false if `roi` is out of such rectangle.
bool getActiveArea( fheroes2::Rect & roi, const int32_t width, const int32_t height )
{
if ( roi.width <= 0 || roi.height <= 0 || roi.x >= width || roi.y >= height )
if ( roi.width <= 0 || roi.height <= 0 || roi.x >= width || roi.y >= height ) {
return false;
}

if ( roi.x < 0 ) {
const int32_t offsetX = -roi.x;
if ( offsetX >= roi.width )
if ( -roi.x >= roi.width ) {
return false;
}

roi.width += roi.x;
roi.x = 0;
roi.width -= offsetX;
}

if ( roi.y < 0 ) {
const int32_t offsetY = -roi.y;
if ( offsetY >= roi.height )
if ( -roi.y >= roi.height ) {
return false;
}

roi.height += roi.y;
roi.y = 0;
roi.height -= offsetY;
}

if ( roi.x + roi.width > width ) {
const int32_t offsetX = roi.x + roi.width - width;
if ( offsetX >= roi.width )
if ( const int32_t offsetX = roi.x + roi.width - width; offsetX > 0 ) {
if ( offsetX >= roi.width ) {
return false;
}

roi.width -= offsetX;
}

if ( roi.y + roi.height > height ) {
const int32_t offsetY = roi.y + roi.height - height;
if ( offsetY >= roi.height )
if ( const int32_t offsetY = roi.y + roi.height - height; offsetY > 0 ) {
if ( offsetY >= roi.height ) {
return false;
}

roi.height -= offsetY;
}

Expand Down Expand Up @@ -448,10 +453,11 @@ namespace

bool isVisible() const override
{
if ( _emulation )
if ( _emulation ) {
return fheroes2::Cursor::isVisible();
else
return fheroes2::Cursor::isVisible() && ( SDL_ShowCursor( SDL_QUERY ) == SDL_ENABLE );
}

return fheroes2::Cursor::isVisible() && ( SDL_ShowCursor( SDL_QUERY ) == SDL_ENABLE );
}

void update( const fheroes2::Image & image, int32_t offsetX, int32_t offsetY ) override
Expand Down Expand Up @@ -1319,9 +1325,6 @@ namespace fheroes2
Display::Display()
: _engine( RenderEngine::create() )
, _cursor( RenderCursor::create() )
, _preprocessing( nullptr )
, _postprocessing( nullptr )
, _renderSurface( nullptr )
{
_disableTransformLayer();
}
Expand Down Expand Up @@ -1369,22 +1372,25 @@ namespace fheroes2
void Display::render( const Rect & roi )
{
Rect temp( roi );
if ( !getActiveArea( temp, width(), height() ) )
if ( !getActiveArea( temp, width(), height() ) ) {
return;

getActiveArea( _prevRoi, width(), height() );
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to call getActiveArea() for _prevRoi here on every call because in line 1407 _prevRoi = temp; and temp is already "updated" by getActiveArea().
The only other place where _prevRoi can get beyond the screen bounds is Display::updateNextRenderRoi(). So I moved getActiveArea( _prevRoi, width(), height() ); there because that method is called much less often than this one.

}

if ( _cursor->isVisible() && _cursor->isSoftwareEmulation() && !_cursor->_image.empty() ) {
const Sprite & cursorImage = _cursor->_image;
const Sprite backup = Crop( *this, cursorImage.x(), cursorImage.y(), cursorImage.width(), cursorImage.height() );
Blit( cursorImage, *this, cursorImage.x(), cursorImage.y() );

if ( !backup.empty() ) {
// ROI must include cursor's area as well, otherwise cursor won't be rendered.
Rect cursorROI( cursorImage.x(), cursorImage.y(), cursorImage.width(), cursorImage.height() );
if ( getActiveArea( cursorROI, width(), height() ) ) {
temp = getBoundaryRect( temp, cursorROI );
}
Rect cursorROI( cursorImage.x(), cursorImage.y(), cursorImage.width(), cursorImage.height() );

if ( _cursor->_keepInScreenArea ) {
cursorROI.x = std::clamp( cursorROI.x, 0, width() - cursorROI.width );
cursorROI.y = std::clamp( cursorROI.y, 0, height() - cursorROI.height );
}

const Sprite backup = Crop( *this, cursorROI.x, cursorROI.y, cursorROI.width, cursorROI.height );
Blit( cursorImage, 0, 0, *this, cursorROI.x, cursorROI.y, cursorROI.width, cursorROI.height );

// ROI must include cursor's area as well, otherwise cursor won't be rendered.
if ( !backup.empty() && getActiveArea( cursorROI, width(), height() ) ) {
temp = getBoundaryRect( temp, cursorROI );
}

// Previous position of cursor must be updated as well to avoid ghost effect.
Expand All @@ -1410,6 +1416,7 @@ namespace fheroes2
void Display::updateNextRenderRoi( const Rect & roi )
{
_prevRoi = getBoundaryRect( _prevRoi, roi );
getActiveArea( _prevRoi, width(), height() );
}

void Display::_renderFrame( const Rect & roi ) const
Expand Down
28 changes: 15 additions & 13 deletions src/engine/screen.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ namespace fheroes2
bool _nearestScaling;
};

class Display : public Image
class Display final : public Image
{
public:
friend class BaseRenderEngine;
Expand Down Expand Up @@ -249,10 +249,10 @@ namespace fheroes2
private:
std::unique_ptr<BaseRenderEngine> _engine;
std::unique_ptr<Cursor> _cursor;
PreRenderProcessing _preprocessing;
PostRenderProcessing _postprocessing;
PreRenderProcessing _preprocessing{ nullptr };
PostRenderProcessing _postprocessing{ nullptr };

uint8_t * _renderSurface;
uint8_t * _renderSurface{ nullptr };

// Previous area drawn on the screen.
Rect _prevRoi;
Expand Down Expand Up @@ -314,17 +314,19 @@ namespace fheroes2
_cursorUpdater = cursorUpdater;
}

void keepInScreenArea( const bool value )
{
_keepInScreenArea = value;
}

protected:
Sprite _image;
bool _emulation;
bool _show;
void ( *_cursorUpdater )();

Cursor()
: _emulation( true )
, _show( false )
, _cursorUpdater( nullptr )
{}
void ( *_cursorUpdater )(){ nullptr };
bool _emulation{ false };
bool _show{ false };
bool _keepInScreenArea{ false };

Cursor() = default;
};

BaseRenderEngine & engine();
Expand Down
12 changes: 9 additions & 3 deletions src/fheroes2/gui/cursor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/***************************************************************************
* fheroes2: https://github.com/ihhub/fheroes2 *
* Copyright (C) 2019 - 2024 *
* Copyright (C) 2019 - 2025 *
* *
* Free Heroes2 Engine: http://sourceforge.net/projects/fheroes2 *
* Copyright (C) 2009 by Andrey Afletdinov <[email protected]> *
Expand Down Expand Up @@ -59,7 +59,11 @@ void Cursor::SetThemes( const int theme, const bool force /* = false */ )
}
const fheroes2::Sprite & spr = fheroes2::AGG::GetICN( icnID, 0xFF & theme );
SetOffset( theme, { ( spr.width() - spr.x() ) / 2, ( spr.height() - spr.y() ) / 2 } );
fheroes2::cursor().update( spr, -_offset.x, -_offset.y );

fheroes2::Cursor & cursor = fheroes2::cursor();
cursor.update( spr, -_offset.x, -_offset.y );
// Force the scroll cursor to stay in screen boundaries.
cursor.keepInScreenArea( theme >= SCROLL_TOP && theme <= SCROLL_TOPLEFT );

// Apply new offset.
const fheroes2::Point & currentPos = LocalEvent::Get().getMouseCursorPos();
Expand All @@ -70,7 +74,9 @@ void Cursor::setCustomImage( const fheroes2::Image & image, const fheroes2::Poin
{
_theme = NONE;

fheroes2::cursor().update( image, -offset.x, -offset.y );
fheroes2::Cursor & cursor = fheroes2::cursor();
cursor.update( image, -offset.x, -offset.y );
cursor.keepInScreenArea( false );

// Immediately apply new mouse offset.
const fheroes2::Point & currentPos = LocalEvent::Get().getMouseCursorPos();
Expand Down
Loading