widelands-dev team mailing list archive

[Klaus Halfmann <klaus.halfmann@xxxxxxxx>]

Checked the code from the top to about 50%, some comments inline.
Did not find the logging, yet.

wil now play a bit with the reduced cache size.

Diff comments:

> 
> === added file 'src/graphic/text/rendered_text.cc'
> --- src/graphic/text/rendered_text.cc	1970-01-01 00:00:00 +0000
> +++ src/graphic/text/rendered_text.cc	2017-05-21 19:15:51 +0000
> @@ -0,0 +1,260 @@
> +/*
> + * Copyright (C) 2017 by the Widelands Development Team
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + *
> + */
> +
> +#include "graphic/text/rendered_text.h"
> +
> +#include <memory>
> +
> +#include "graphic/graphic.h"
> +#include "graphic/text_layout.h"
> +
> +namespace UI {
> +// RenderedRect
> +RenderedRect::RenderedRect(const Recti& init_rect,
> +                           std::shared_ptr<const Image> init_image,
> +                           bool visited,
> +                           const RGBColor& color,
> +                           bool is_background_color_set,
> +                           DrawMode init_mode)
> +   : rect_(init_rect),
> +     transient_image_(init_image),
> +     permanent_image_(nullptr),
> +     visited_(visited),
> +     background_color_(color),
> +     is_background_color_set_(is_background_color_set),
> +     mode_(init_mode) {
> +}
> +RenderedRect::RenderedRect(const Recti& init_rect,
> +                           const Image* init_image,
> +                           bool visited,
> +                           const RGBColor& color,
> +                           bool is_background_color_set,
> +                           DrawMode init_mode)
> +   : rect_(init_rect),
> +     transient_image_(nullptr),
> +     permanent_image_(init_image),
> +     visited_(visited),
> +     background_color_(color),
> +     is_background_color_set_(is_background_color_set),
> +     mode_(init_mode) {
> +}
> +
> +RenderedRect::RenderedRect(const Recti& init_rect, const Image* init_image)
> +   : RenderedRect(init_rect, init_image, false, RGBColor(0, 0, 0), false, DrawMode::kTile) {
> +}
> +RenderedRect::RenderedRect(const Recti& init_rect, const RGBColor& color)
> +   : RenderedRect(init_rect, nullptr, false, color, true, DrawMode::kTile) {

better replace true / false by kBackGroundColorSet and !kBackGroundColorSet or such

> +}
> +RenderedRect::RenderedRect(std::shared_ptr<const Image> init_image)
> +   : RenderedRect(Recti(0, 0, init_image->width(), init_image->height()),
> +                  init_image,
> +                  false,
> +                  RGBColor(0, 0, 0),
> +                  false,
> +                  DrawMode::kBlit) {
> +}
> +RenderedRect::RenderedRect(const Image* init_image)
> +   : RenderedRect(Recti(0, 0, init_image->width(), init_image->height()),
> +                  init_image,
> +                  false,
> +                  RGBColor(0, 0, 0),
> +                  false,
> +                  DrawMode::kBlit) {
> +}
> +
> +const Image* RenderedRect::image() const {
> +	assert(permanent_image_ == nullptr || transient_image_.get() == nullptr);
> +	return permanent_image_ == nullptr ? transient_image_.get() : permanent_image_;
> +}
> +
> +int RenderedRect::x() const {
> +	return rect_.x;
> +}
> +
> +int RenderedRect::y() const {
> +	return rect_.y;
> +}
> +
> +int RenderedRect::width() const {
> +	return rect_.w;
> +}
> +int RenderedRect::height() const {
> +	return rect_.h;
> +}
> +
> +void RenderedRect::set_origin(const Vector2i& new_origin) {
> +	rect_.x = new_origin.x;
> +	rect_.y = new_origin.y;
> +}
> +void RenderedRect::set_visited() {
> +	visited_ = true;
> +}
> +bool RenderedRect::was_visited() const {
> +	return visited_;
> +}
> +
> +bool RenderedRect::has_background_color() const {
> +	return is_background_color_set_;
> +}
> +const RGBColor& RenderedRect::background_color() const {
> +	return background_color_;
> +}
> +
> +RenderedRect::DrawMode RenderedRect::mode() const {
> +	return mode_;
> +}
> +
> +// RenderedText
> +int RenderedText::width() const {
> +	int result = 0;
> +	for (const auto& rect : rects) {
> +		result = std::max(result, rect->x() + rect->width());
> +	}
> +	return result;
> +}
> +int RenderedText::height() const {
> +	int result = 0;
> +	for (const auto& rect : rects) {
> +		result = std::max(result, rect->y() + rect->height());
> +	}
> +	return result;
> +}
> +
> +void RenderedText::draw(RenderTarget& dst,
> +                        const Vector2i& position,
> +                        const Recti& region,
> +                        Align align,
> +                        CropMode cropmode) const {
> +
> +	// Un-const the position and adjust for alignment according to region width
> +	Vector2i aligned_pos(position.x, position.y);
> +
> +	// For cropping images that don't fit
> +	int offset_x = 0;
> +	if (cropmode == CropMode::kHorizontal) {
> +		UI::correct_for_align(align, width(), &aligned_pos);
> +		if (align != UI::Align::kLeft) {
> +			for (const auto& rect : rects) {
> +				offset_x = std::min(region.w - rect->width(), offset_x);
> +			}
> +			if (align == UI::Align::kCenter) {
> +				offset_x /= 2;
> +			}
> +		}
> +	} else {

// CropMode::kVertical

> +		aligned_pos.x -= region.x;
> +		aligned_pos.y -= region.y;
> +		UI::correct_for_align(align, region.w, &aligned_pos);
> +	}
> +
> +	// Blit the rects

This should become a function blit_rects, I already lost track reading till here

> +	for (const auto& rect : rects) {
> +		const Vector2i blit_point(aligned_pos.x + rect->x(), aligned_pos.y + rect->y());
> +
> +		// Draw Solid background Color
> +		if (rect->has_background_color()) {
> +#ifndef NDEBUG
> +			const int maximum_size = kMinimumSizeForTextures;
> +#else
> +			const int maximum_size = g_gr->max_texture_size();
> +#endif
> +			// NOCOM Replace 'test' with 'maximum_size' when testing is done.
> +			const int test = 4;
> +			const int tile_width = std::min(test, rect->width());
> +			const int tile_height = std::min(test, rect->height());
> +			for (int tile_x = blit_point.x; tile_x + tile_width <= blit_point.x + rect->width();
> +			     tile_x += tile_width) {
> +				for (int tile_y = blit_point.y; tile_y + tile_height <= blit_point.y + rect->height();
> +				     tile_y += tile_height) {
> +					dst.fill_rect(
> +					   Recti(tile_x, tile_y, tile_width, tile_height), rect->background_color());
> +				}
> +			}
> +		}
> +
> +		if (rect->image() != nullptr) {
> +			switch (rect->mode()) {
> +			// Draw a foreground texture
> +			case RenderedRect::DrawMode::kBlit: {
> +				switch (cropmode) {
> +				case CropMode::kRenderTarget:
> +					// dst will handle any cropping
> +					dst.blit(blit_point, rect->image());
> +					break;
> +				case CropMode::kHorizontal:
> +					blit_cropped(dst, offset_x, position, blit_point, *rect, region, align);
> +				}
> +			} break;
> +			// Draw a background image (tiling)
> +			case RenderedRect::DrawMode::kTile:
> +				dst.tile(
> +				   Recti(blit_point, rect->width(), rect->height()), rect->image(), Vector2i(0, 0));
> +				break;
> +			}
> +		}
> +	}
> +}
> +
> +void RenderedText::draw(RenderTarget& dst, const Vector2i& position, UI::Align align) const {
> +	draw(dst, position, Recti(0, 0, width(), height()), align);
> +}
> +
> +// Crop horizontally if it doesn't fit
> +void RenderedText::blit_cropped(RenderTarget& dst,
> +                                int offset_x,
> +                                const Vector2i& position,
> +                                const Vector2i& blit_point,
> +                                const RenderedRect& rect,
> +                                const Recti& region,
> +                                Align align) const {
> +
> +	int blit_width = rect.width();
> +	int cropped_left = 0;
> +	if (align != UI::Align::kLeft) {
> +		if (rect.x() + rect.width() + offset_x <= region.x) {
> +			// Falls off the left-hand side
> +			return;
> +		}
> +		if (rect.x() + offset_x < 0) {
> +			// Needs cropping
> +			blit_width = rect.width() + offset_x + rect.x() - region.x;
> +			cropped_left = rect.width() - blit_width;
> +		}
> +	}
> +
> +	if (align != UI::Align::kRight) {
> +		if (rect.x() + rect.width() - offset_x > region.w - region.x) {
> +			blit_width = region.w - rect.x() - offset_x;
> +		}
> +	}
> +
> +	// Don't blit tiny or negative width
> +	if (blit_width < 3) {
> +		return;
> +	}
> +
> +	dst.blitrect(
> +	   Vector2i(cropped_left > 0 ?
> +	               position.x + region.x - (align == UI::Align::kRight ? region.w : region.w / 2) :
> +	               blit_point.x,
> +	            blit_point.y),
> +	   rect.image(), Recti(cropped_left > 0 ? cropped_left : 0, region.y, blit_width, region.h));
> +}
> +
> +}  // namespace UI
> 
> === added file 'src/graphic/text/rendered_text.h'
> --- src/graphic/text/rendered_text.h	1970-01-01 00:00:00 +0000
> +++ src/graphic/text/rendered_text.h	2017-05-21 19:15:51 +0000
> @@ -0,0 +1,156 @@
> +/*
> + * Copyright (C) 2017 by the Widelands Development Team
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + *
> + */
> +
> +#ifndef WL_GRAPHIC_TEXT_RENDERED_TEXT_H
> +#define WL_GRAPHIC_TEXT_RENDERED_TEXT_H
> +
> +#include <memory>
> +#include <vector>
> +
> +#include "base/rect.h"
> +#include "base/vector.h"
> +#include "graphic/image.h"
> +#include "graphic/rendertarget.h"
> +#include "graphic/texture.h"
> +
> +namespace UI {
> +
> +/// A rectangle that contains blitting information for rendered text.

Perhaps we need mor infor about blitting / cropping with some examples.

e.g. kBlit : copy Text pixel in one opertation
     kTile : compose text from mutiple rectangles (cropeed) horizontal or vertical

> +class RenderedRect {
> +public:
> +	/// Whether the RenderedRect's image should be blitted once or tiled
> +	enum class DrawMode { kBlit, kTile };
> +
> +private:
> +	// The image is managed by a transient cache
> +	RenderedRect(const Recti& init_rect,
> +	             std::shared_ptr<const Image> init_image,
> +	             bool visited,
> +	             const RGBColor& color,
> +	             bool is_background_color_set,
> +	             DrawMode init_mode);
> +	// The image is managed by a pernament cache
> +	RenderedRect(const Recti& init_rect,
> +	             const Image* init_image,
> +	             bool visited,
> +	             const RGBColor& color,
> +	             bool is_background_color_set,
> +	             DrawMode init_mode);
> +
> +public:
> +	/// RenderedRect will contain a background image that should be tiled
> +	RenderedRect(const Recti& init_rect, const Image* init_image);
> +
> +	/// RenderedRect will contain a background color that should be tiled
> +	RenderedRect(const Recti& init_rect, const RGBColor& color);
> +
> +	/// RenderedRect will contain a normal image that is managed by a transient cache.
> +	/// Use this if the image is managed by an instance of TextureCache.
> +	RenderedRect(std::shared_ptr<const Image> init_image);
> +
> +	/// RenderedRect will contain a normal image that is managed by a permanent cache.
> +	/// Use this if the image is managed by g_gr->images().
> +	RenderedRect(const Image* init_image);
> +	~RenderedRect() {
> +	}
> +
> +	/// An image to be blitted. Can be nullptr.
> +	const Image* image() const;
> +
> +	/// The x position of the rectangle
> +	int x() const;
> +	/// The y position of the rectangle
> +	int y() const;
> +	/// The width of the rectangle
> +	int width() const;
> +	/// The height of the rectangle
> +	int height() const;
> +
> +	/// Change x and y position of the rectangle.
> +	void set_origin(const Vector2i& new_origin);
> +
> +	/// Set that this rectangle was already visited by the font renderer. Needed by the font renderer
> +	/// for correct positioning.
> +	void set_visited();
> +	/// Whether this rectangle was already visited by the font renderer
> +	bool was_visited() const;
> +
> +	/// Whether this rectangle contains a background color
> +	bool has_background_color() const;
> +	/// This rectangle's background color
> +	const RGBColor& background_color() const;
> +
> +	/// Whether the RenderedRect's image should be blitted once or tiled
> +	DrawMode mode() const;
> +
> +private:
> +	Recti rect_;
> +	// We have 2 image objects depending on the caching situation - only use one of them at the same
> +	// time.
> +	std::shared_ptr<const Image> transient_image_;  // Shared ownership, managed by a transient cache
> +	const Image* permanent_image_;                  // Not owned, managed by a permanent cache
> +	bool visited_;
> +	const RGBColor background_color_;
> +	const bool is_background_color_set_;
> +	const DrawMode mode_;
> +};
> +
> +struct RenderedText {
> +	/// RenderedRects that can be drawn on screen
> +	std::vector<std::unique_ptr<RenderedRect>> rects;
> +
> +	/// The width occupied  by all rects in pixels.
> +	int width() const;
> +	/// The height occupied  by all rects in pixels.
> +	int height() const;
> +
> +	enum class CropMode {
> +		kRenderTarget,  // The RenderTarget will handle all cropping
> +		kHorizontal     // The draw() method will handle horizontal cropping
> +	};
> +
> +	/// Draw the rects. 'position', 'region' and 'align' are used to control the overall drawing
> +	/// position and cropping.
> +	/// For 'cropmode', use kRenderTarget if you wish the text to fill the whole RenderTarget, e.g.
> +	/// for scrolling panels. Use kHorizontal for horizontal cropping in smaller elements, e.g. table
> +	/// cells.
> +	void draw(RenderTarget& dst,
> +	          const Vector2i& position,
> +	          const Recti& region,
> +	          UI::Align align = UI::Align::kLeft,
> +	          CropMode cropmode = CropMode::kRenderTarget) const;
> +
> +	/// Draw the rects without cropping. 'position' and 'align' are used to control the overall
> +	/// drawing position
> +	void draw(RenderTarget& dst, const Vector2i& position, UI::Align align = UI::Align::kLeft) const;
> +
> +private:
> +	/// Helper function for CropMode::kHorizontal
> +	void blit_cropped(RenderTarget& dst,
> +	                  int offset_x,
> +	                  const Vector2i& position,
> +	                  const Vector2i& blit_point,
> +	                  const RenderedRect& rect,
> +	                  const Recti& region,
> +	                  Align align) const;
> +};
> +
> +}  // namespace UI
> +
> +#endif  // end of include guard: WL_GRAPHIC_TEXT_RENDERED_TEXT_H
> 
> === modified file 'src/graphic/text/rt_render.cc'
> --- src/graphic/text/rt_render.cc	2017-05-03 07:24:06 +0000
> +++ src/graphic/text/rt_render.cc	2017-05-21 19:15:51 +0000
> @@ -599,19 +668,24 @@
>  public:
>  	NewlineNode(NodeStyle& ns) : RenderNode(ns) {
>  	}
> -	uint16_t height() override {
> +
> +	std::string debug_info() const override {
> +		return "nl";
> +	}
> +
> +	uint16_t height() const override {
>  		return 0;
>  	}
> -	uint16_t width() override {
> +	uint16_t width() const override {
>  		return INFINITE_WIDTH;
>  	}
> -	uint16_t hotspot_y() override {
> +	uint16_t hotspot_y() const override {
>  		return 0;
>  	}
> -	Texture* render(TextureCache* /* texture_cache */) override {
> +	UI::RenderedText* render(TextureCache* /* texture_cache */) override {
>  		NEVER_HERE();
>  	}
> -	bool is_non_mandatory_space() override {
> +	bool is_non_mandatory_space() const override {

Found no comment for this I would expect perhaps:
/// return true when the spaces (and beginning and end) must be rendered in any case.
/// return false when spaces at start and end can be omnitted in some/what ? cases

>  		return true;
>  	}
>  };


-- 
https://code.launchpad.net/~widelands-dev/widelands/fh1-multitexture/+merge/323903
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fh1-multitexture.