Skip to content
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

Jump to bottom

Fix high FPS while right clicking on items in lists #7647

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Fix high FPS while right clicking on items in lists #7647

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e335ab2
Fix high FPS while right clicking on items in lists
ihhub Aug 27, 2023
5c2d85b
Merge branch 'master' into right_click_high_fps
ihhub Sep 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/fheroes2/gui/interface_list.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,16 +419,16 @@ namespace Interface

const fheroes2::Point & mousePos = le.GetMouseCursor();
if ( rtAreaItems & mousePos ) { // within our rectangle
needRedraw = true;

const int id = ( mousePos.y - rtAreaItems.y ) * maxItems / rtAreaItems.height + _topId;

if ( id < _size() ) {
Item & item = ( *content )[static_cast<size_t>( id )]; // id is always >= 0
const int32_t offsetY = ( id - _topId ) * rtAreaItems.height / maxItems;

if ( ActionListCursor( item, mousePos ) )
if ( ActionListCursor( item, mousePos ) ) {
needRedraw = true;
return true;
}

if ( le.MouseClickLeft( rtAreaItems ) ) {
if ( id == _currentId ) {
Expand All @@ -438,15 +438,15 @@ namespace Interface
_currentId = id;
ActionListSingleClick( item, mousePos, rtAreaItems.x, rtAreaItems.y + offsetY );
}
needRedraw = true;

return true;
}
else if ( le.MousePressRight( rtAreaItems ) ) {
ActionListPressRight( item, mousePos, rtAreaItems.x, rtAreaItems.y + offsetY );
return true;
Copy link
Collaborator

@Districh-ru Districh-ru Aug 27, 2023 •
edited

Choose a reason for hiding this comment

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

After some checks for the cause of high FPS on every right mouse button hold on empty field (castle captain, secondary skill) in Kingdom Overview I found that this return value (true) is used to set redraw to true. In other places it seems that this state is used for the same purposes: to redraw screen or update data after changes.
But holding the right mouse button above the list item should not cause the change of any data. Therefore possibly we should return false here. What do you think, @ihhub?

PS High FPS on right mouse press on any empty artifact/unit in heroes meeting, hero and castle screens can be fixed

by changing:

fheroes2/src/fheroes2/army/army_bar.cpp

Lines 626 to 630 in e335ab2

}
}
return true;
}

and

fheroes2/src/fheroes2/resource/artifact.cpp

Lines 1240 to 1244 in e335ab2

}
}
return true;
}

to:

        }

        return true;
    }

    return false;
}

The solution is taken from:

fheroes2/src/fheroes2/gui/skill_bar.cpp

Lines 255 to 268 in e335ab2

bool SecondarySkillsBar::ActionBarRightMouseHold( Skill::Secondary & skill )
{
if ( skill.isValid() ) {
if ( can_change ) {
skill.Reset();
}
else {
fheroes2::SecondarySkillDialogElement( skill, _hero ).showPopup( Dialog::ZERO );
}
return true;
}
return false;
}

Copy link
Owner Author

Choose a reason for hiding this comment

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

Hi @Districh-ru , I think we should also return a boolean value from ActionListPressRight as well to be consistent with the code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi, @ihhub, you are right, then ActionListPressRight() will return true if redraw is needed and we will return it from QueueEventProcessing().

}
}

needRedraw = false;
}

return false;
Expand Down