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.

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

improve tooltip placement on rh panel #8908

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 7 additions & 2 deletions src/floating_label.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ rect floating_label::get_bg_rect(const rect& text_rect) const
};
}

void floating_label::clear_texture()
{
tex_.reset();
}

bool floating_label::create_texture()
{
if(video::headless()) {
Expand Down Expand Up @@ -180,7 +185,7 @@ void floating_label::update(int time)
// Invalidate new draw loc in preparation
draw_manager::invalidate_region(get_bg_rect(draw_loc));

DBG_FT << "updating floating label from " << screen_loc_ << " to " << draw_loc;
// DBG_FT << "updating floating label from " << screen_loc_ << " to " << draw_loc;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you comment this out? If you don't need it, just remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh. I commented those out because they were spewing 60 messages each every second to the log I was trying to watch, and I just forgot to put them back. Actually, I suspect someone else just forgot to remove them when they were no longer needed so they should probably be removed, but I'm hesitant to do since I'm not sure. I can uncomment them or remove them. ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Uncomment them, since they are also uncommented in master.


screen_loc_ = draw_loc;
alpha_ = new_alpha;
Expand All @@ -206,7 +211,7 @@ void floating_label::draw()
return;
}

DBG_FT << "drawing floating label to " << screen_loc_;
// DBG_FT << "drawing floating label to " << screen_loc_;
Copy link
Contributor

@babaissarkar babaissarkar Jun 26, 2024

Choose a reason for hiding this comment

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

Unnecessary commented out code.


// Clip if appropriate.
auto clipper = draw::reduce_clip(clip_rect_);
Expand Down
2 changes: 2 additions & 0 deletions src/floating_label.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ class floating_label
*/
bool create_texture();

void clear_texture(); // for testing

/** Return the size of the label in drawing coordinates */
SDL_Point get_draw_size() const
{
Expand Down
74 changes: 68 additions & 6 deletions src/tooltips.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ namespace {

static const int font_size = font::SIZE_SMALL;
static const int text_width = 400;
static const double height_fudge = 0.95; // An artificial "border" to keep tip text from crowding lower edge of viewing area

struct tooltip
{
Expand Down Expand Up @@ -60,16 +61,56 @@ void tooltip::init_label()
rect game_canvas = video::game_canvas();
unsigned int border = 10;

rect huge;
huge.h=1000000;
huge.w=1000000;

label.set_font_size(font_size);
label.set_color(font::NORMAL_COLOR);
label.set_clip_rect(game_canvas);
label.set_width(text_width);
label.set_clip_rect(huge);
label.set_width(text_width); // If tooltip will be too tall for game_canvas, this could be scaled up appropriately
label.set_alignment(font::LEFT_ALIGN);
label.set_bg_color(bgcolor);
label.set_border_size(border);

label.create_texture();

point lsize = label.get_draw_size();
int new_text_width = text_width * static_cast<float>(lsize.y)/game_canvas.h; // If necessary, scale width to reduce height while preserving area of label
while((lsize.y > game_canvas.h*height_fudge) && (lsize.x < game_canvas.w)) {
// Scaling the tip to reduce height is hard, since making a texture wider is no guarantee that there will be fewer lines of text:
//
// This block of text is just
// as tall as the other one.
//
// This block of text is just as tall as the other
// one.
//
// Creating this over and over may not be the most efficient route, but it will work and will be quite rare (tip taller than screen).
bool wont_fit = false;
if(new_text_width>game_canvas.w) {
new_text_width=game_canvas.w;
wont_fit = true;
}
DBG_FT << "lsize.x,y = " << lsize.x << "," << lsize.y << ", new_text_width = " << new_text_width;

label.set_width(new_text_width);
label.clear_texture();
label.create_texture();

lsize = label.get_draw_size();
DBG_FT << "new label lsize.x,y = " << lsize.x << "," << lsize.y;
if(wont_fit) {
break;
}
new_text_width *= 1.3;
}
// I don't know if it's strictly necessary to create the texture yet again just to make sure the clip_rect is set to game_canvas
// but it seems like the safe course of action.
label.set_clip_rect(game_canvas);
label.clear_texture();
label.create_texture();

update_label_pos();
}

Expand All @@ -80,21 +121,42 @@ void tooltip::update_label_pos()
point lsize = label.get_draw_size();
loc = {0, 0, lsize.x, lsize.y};

// See if there is enough room to fit it above the tip area
DBG_FT << "\nupdate_label_pos() Start: loc = " << loc.x << "," << loc.y << " origin = " << origin.x << "," << origin.y;

if(origin.y > loc.h) {
// There is enough room to fit it above the tip area
loc.y = origin.y - loc.h;
} else {
DBG_FT << "\tAbove: loc = " << loc.x << "," << loc.y << " origin = " << origin.x << "," << origin.y;
} else if((origin.y + origin.h + loc.h) <= game_canvas.h*height_fudge) {
// There is enough room to fit it below the tip area
loc.y = origin.y + origin.h;
DBG_FT << "\tBelow: loc = " << loc.x << "," << loc.y << " origin = " << origin.x << "," << origin.y;
} else if(((origin.y + origin.h/2 - loc.h/2) >= 0) &&
((origin.y + origin.h/2 + loc.h/2) <= game_canvas.h*height_fudge)) {
// There is enough room to center it at the tip area
loc.y = origin.y + origin.h/2 - loc.h/2;
DBG_FT << "\tCenter: loc = " << loc.x << "," << loc.y << " origin = " << origin.x << "," << origin.y;
} else if(loc.h <= game_canvas.h*0.95) {
// There is enough room to center it
loc.y = game_canvas.h/2 - loc.h/2;
DBG_FT << "\tScreen Center: loc = " << loc.x << "," << loc.y << " origin = " << origin.x << "," << origin.y;
} else {
// It doesn't fit
loc.y = 0;
DBG_FT << "\tToo big: loc = " << loc.x << "," << loc.y << " origin = " << origin.x << "," << origin.y;
}

DBG_FT << "\tBefore x adjust: loc.x,y,w,h = " << loc.x << "," << loc.y << "," << loc.w << "," << loc.h << " origin = " << origin.x << "," << origin.y;
// Try to keep it within the screen
loc.x = origin.x;
if(loc.x + loc.w > game_canvas.w) {
loc.x = game_canvas.w - loc.w;
}
if(loc.x < 0) {
loc.x = 0;
} else if(loc.x + loc.w > game_canvas.w) {
loc.x = game_canvas.w - loc.w;
}

DBG_FT << "\tFinal: loc.x,y,w,h = " << loc.x << "," << loc.y << "," << loc.w << "," << loc.h << " origin = " << origin.x << "," << origin.y;
label.set_position(loc.x, loc.y);
}

Expand Down