From e44129fb919c06c848480f3ae48fde225cedca30 Mon Sep 17 00:00:00 2001 From: Tommy Date: Thu, 30 Jun 2022 13:22:18 +1200 Subject: [PATCH] Address some slightly less straightforward TODOs --- src/display.cpp | 67 +++++-------------------------------- src/gui/core/canvas.cpp | 13 ++++--- src/help/help_text_area.cpp | 6 ---- src/show_dialog.cpp | 10 +++--- src/video.cpp | 4 --- src/widgets/textbox.cpp | 5 ++- 6 files changed, 20 insertions(+), 85 deletions(-) diff --git a/src/display.cpp b/src/display.cpp index b6f93af3a77..fb5de36d62b 100644 --- a/src/display.cpp +++ b/src/display.cpp @@ -1051,7 +1051,6 @@ std::vector display::get_fog_shroud_images(const map_location& loc, ima return res; } -// TODO: highdpi - verify these get scaled correctly void display::get_terrain_images(const map_location& loc, const std::string& timeid, TERRAIN_TYPE terrain_type) { terrain_image_vector_.clear(); @@ -1791,7 +1790,6 @@ void display::draw_minimap() } } - // TODO: highdpi - does this really need to set a clipping area? auto clipper = draw::set_clip(area); // Draw the minimap background. @@ -1836,12 +1834,6 @@ void display::draw_minimap() view_h + 2 }; - // SDL 2.0.10's render batching changes result in the - // surface's clipping rectangle being overridden even if - // no render clipping rectangle set operaton was queued, - // so let's not use the render API to draw the rectangle. - - // TODO: highdpi - is the above still relevant? It doesn't seem to be. draw::rect(outline_rect, 255, 255, 255); } @@ -2734,44 +2726,6 @@ void display::draw_hex(const map_location& loc) } -// TODO: highdpi - why is there all this faff to deal with textures that are fill of transparency? Just don't make your textures full of transparency. This should not be a thing. -// Usage of this function has been removed. -// If this turns out to raise no problems, remove the function too. -#if 0 -void display::draw_image_for_report(surface& img, const SDL_Rect& rect) -{ - SDL_Rect visible_area = get_non_transparent_portion(img); - SDL_Rect target = rect; - if(visible_area.x != 0 || visible_area.y != 0 || visible_area.w != img->w || visible_area.h != img->h) { - if(visible_area.w == 0 || visible_area.h == 0) { - return; - } - - if(visible_area.w > rect.w || visible_area.h > rect.h) { - img = get_surface_portion(img,visible_area); - img = scale_surface(img,rect.w,rect.h); - visible_area.x = 0; - visible_area.y = 0; - visible_area.w = img->w; - visible_area.h = img->h; - } else { - target.x = rect.x + (rect.w - visible_area.w)/2; - target.y = rect.y + (rect.h - visible_area.h)/2; - target.w = visible_area.w; - target.h = visible_area.h; - } - - screen_.blit_surface(target.x, target.y, img, &visible_area, nullptr); - } else { - if(img->w != rect.w || img->h != rect.h) { - img = scale_surface(img,rect.w,rect.h); - } - - screen_.blit_surface(img, &target); - } -} -#endif - /** * Redraws the specified report (if anything has changed). * If a config is not supplied, it will be generated via @@ -2801,36 +2755,34 @@ void display::refresh_report(const std::string& report_name, const config * new_ SDL_Rect &rect = reportRects_[report_name]; const SDL_Rect &new_rect = item->location(screen_.draw_area()); - texture &tex = reportSurfaces_[report_name]; + texture &bg_restore = reportSurfaces_[report_name]; config &report = reports_[report_name]; // Report and its location is unchanged since last time. Do nothing. - if (tex && rect == new_rect && report == *new_cfg) { + if (bg_restore && rect == new_rect && report == *new_cfg) { return; } // Update the config in reports_. report = *new_cfg; - // TODO: highdpi - should this really be drawn if rect != new_rect? That's how the old code was. - if (tex) { - draw::blit(tex, rect); + // TODO: highdpi - remove background restorer + if (bg_restore) { + draw::blit(bg_restore, rect); } // If the rectangle has just changed, assign the surface to it - if (!tex || new_rect != rect) + if (!bg_restore || new_rect != rect) { - tex.reset(); + bg_restore.reset(); rect = new_rect; - // TODO: highdpi - i have no idea why this is neccesary, as the only report backgrounds i have seen were just black. Maybe it can just stop doing this? - // If the rectangle is present, and we are blitting text, // then we need to backup the surface. // (Images generally won't need backing up, // unless they are transparent, but that is done later). if (rect.w > 0 && rect.h > 0) { - tex = texture(screen_.read_pixels_low_res(&rect)); + bg_restore = texture(screen_.read_pixels_low_res(&rect)); if (!reportSurfaces_[report_name]) { ERR_DP << "Could not backup background for report!" << std::endl; } @@ -2944,7 +2896,6 @@ void display::refresh_report(const std::string& report_name, const config * new_ continue; } - // TODO: highdpi - set size independently of image if (area.w < img.w() && image_count) { // We have more than one image, and this one doesn't fit. img = image::get_texture(game_config::images::ellipsis); @@ -2953,8 +2904,6 @@ void display::refresh_report(const std::string& report_name, const config * new_ if (img.w() < area.w) area.w = img.w(); if (img.h() < area.h) area.h = img.h(); - // TODO: highdpi - this was crazy so i nixed it - //draw_image_for_report(img, area); draw::blit(img, area); ++image_count; diff --git a/src/gui/core/canvas.cpp b/src/gui/core/canvas.cpp index e7dce31ef9e..a8a75921a97 100644 --- a/src/gui/core/canvas.cpp +++ b/src/gui/core/canvas.cpp @@ -318,13 +318,12 @@ void image_shape::draw(wfl::map_formula_callable& variables) local_variables.add("image_width", wfl::variant(w ? w : tex.w())); local_variables.add("image_height", wfl::variant(h ? h : tex.h())); - // TODO: highdpi - why are these called "clip"? - const unsigned clip_x = x_(local_variables); - const unsigned clip_y = y_(local_variables); + const int x = x_(local_variables); + const int y = y_(local_variables); - // TODO: highdpi - what are these for? They are never used anywhere else. - local_variables.add("clip_x", wfl::variant(clip_x)); - local_variables.add("clip_y", wfl::variant(clip_y)); + // used in gui/dialogs/story_viewer.cpp + local_variables.add("clip_x", wfl::variant(x)); + local_variables.add("clip_y", wfl::variant(y)); // Execute the provided actions for this context. wfl::variant(variables.fake_ptr()).execute_variant(actions_formula_.evaluate(local_variables)); @@ -333,7 +332,7 @@ void image_shape::draw(wfl::map_formula_callable& variables) if (!w) { w = tex.w(); } if (!h) { h = tex.h(); } - const SDL_Rect dst_rect { static_cast(clip_x), static_cast(clip_y), w, h }; + const SDL_Rect dst_rect { x, y, w, h }; // What to do with the image depends on whether we need to tile it or not. switch(resize_mode_) { diff --git a/src/help/help_text_area.cpp b/src/help/help_text_area.cpp index c5ddbd8cd3d..06324482f55 100644 --- a/src/help/help_text_area.cpp +++ b/src/help/help_text_area.cpp @@ -558,12 +558,6 @@ void help_text_area::draw_contents() it->rect.w - i * 2, it->rect.h - i * 2 }; - - // SDL 2.0.10's render batching changes result in the - // surface's clipping rectangle being overridden even if - // no render clipping rectangle set operaton was queued, - // so let's not use the render API to draw the rectangle. - // TODO: highdpi - is the above still relevant? draw::fill(draw_rect, 0, 0, 0, 0); ++dst.x; ++dst.y; diff --git a/src/show_dialog.cpp b/src/show_dialog.cpp index 30204a050b3..fbfbdab7837 100644 --- a/src/show_dialog.cpp +++ b/src/show_dialog.cpp @@ -319,12 +319,10 @@ void dialog_frame::draw_background() } if (dialog_style_.blur_radius) { - SDL_Rect r = dim_.exterior; - surface surf = video_.read_pixels_low_res(&r); - surf = blur_surface(surf, dialog_style_.blur_radius); - // TODO: highdpi - converting this to texture every time is not ideal, but i also suspect this is not actually used anywhere, and all this is slated for removal anyway. - WRN_DP << "deprecated very slow blur" << std::endl; - draw::blit(texture(surf), r); + // This is no longer used by anything. + // The only thing that uses dialog_frame is help/help.cpp, + // and it uses the default style with no blur. + ERR_DP << "GUI1 dialog_frame blur has been removed" << std::endl; } if (!bg_) { diff --git a/src/video.cpp b/src/video.cpp index 1255350b8da..c773ed9f95c 100644 --- a/src/video.cpp +++ b/src/video.cpp @@ -72,10 +72,6 @@ void trigger_full_redraw() SDL_Event event; event.type = SDL_WINDOWEVENT; event.window.event = SDL_WINDOWEVENT_EXPOSED; - // TODO: highdpi - double check whether anything actually does need a resize event - //event.window.event = SDL_WINDOWEVENT_RESIZED; - //event.window.data1 = (*drawingSurface).h; - //event.window.data2 = (*drawingSurface).w; for(const auto& layer : draw_layers) { layer->handle_window_event(event); diff --git a/src/widgets/textbox.cpp b/src/widgets/textbox.cpp index 251a67e5195..78af5b75274 100644 --- a/src/widgets/textbox.cpp +++ b/src/widgets/textbox.cpp @@ -206,6 +206,8 @@ void textbox::draw_contents() const int endx = char_x_[end]; const int endy = char_y_[end]; + auto clipper = draw::set_clip(loc); + while(starty <= endy) { const std::size_t right = starty == endy ? endx : text_image_.w(); if(right <= std::size_t(startx)) { @@ -217,9 +219,6 @@ void textbox::draw_contents() , right - startx , line_height_); - // TODO: highdpi - this seems excessive - auto clipper = draw::set_clip(loc); - draw::fill(rect, 0, 0, 160, 140); starty += int(line_height_);