Review: Approve
some minor nits in the code.
This is way better encapsulated than before, but it still does not make me all
warm and fuzzy inside - it feels weird that the animation passes out its
calculated source rect, just to get it back in the blitting call. I do not have
any more immediate suggestions how to make this better though.
Diff comments:
>
> === modified file 'src/graphic/animation.cc'
> --- src/graphic/animation.cc 2016-10-26 10:56:02 +0000
> +++ src/graphic/animation.cc 2016-11-18 17:52:33 +0000
> @@ -256,20 +257,35 @@
> }
> }
>
> +Rectf NonPackedAnimation::source_rectangle(int percent_from_bottom) const {
const int per...
> + ensure_graphics_are_loaded();
> + float height = percent_from_bottom * frames_[0]->height() / 100;
> + return Rectf(0.f, frames_[0]->height() - height, frames_[0]->width(),
> height);
> +}
> +
> +Rectf NonPackedAnimation::destination_rectangle(const Vector2f& position,
> + const Rectf& source_rect,
> + float scale) const {
const float
> + ensure_graphics_are_loaded();
> + return Rectf(position.x - (hotspot_.x - source_rect.x / scale_) * scale,
> + position.y - (hotspot_.y - source_rect.y / scale_) * scale,
> + source_rect.w * scale / scale_, source_rect.h * scale /
> scale_);
> +}
> +
> void NonPackedAnimation::blit(uint32_t time,
> - const Rectf& dstrc,
> - const Rectf& srcrc,
> + const Rectf& source_rect,
> + const Rectf& destination_rect,
> const RGBColor* clr,
> Surface* target) const {
> + ensure_graphics_are_loaded();
> assert(target);
> -
> const uint32_t idx = current_frame(time);
> assert(idx < nr_frames());
> -
> if (!hasplrclrs_ || clr == nullptr) {
> - target->blit(dstrc, *frames_.at(idx), srcrc, 1.,
> BlendMode::UseAlpha);
> + target->blit(destination_rect, *frames_.at(idx), source_rect,
> 1., BlendMode::UseAlpha);
> } else {
> - target->blit_blended(dstrc, *frames_.at(idx),
> *pcmasks_.at(idx), srcrc, *clr);
> + target->blit_blended(
> + destination_rect, *frames_.at(idx), *pcmasks_.at(idx),
> source_rect, *clr);
> }
> }
>
>
> === modified file 'src/graphic/animation.h'
> --- src/graphic/animation.h 2016-10-25 08:14:28 +0000
> +++ src/graphic/animation.h 2016-11-18 17:52:33 +0000
> @@ -57,8 +58,17 @@
> }
>
> /// The dimensions of this animation.
Fix comment.
> - virtual uint16_t width() const = 0;
> - virtual uint16_t height() const = 0;
> + virtual float height() const = 0;
I am not entirely sure, but I *think* this can be in a protected section now.
> +
> + /// The size of the animation source images. Use 'percent_from_bottom'
> to crop the animation.
Units?
> + virtual Rectf source_rectangle(int percent_from_bottom) const = 0;
> +
> + /// Calculates the destination rectangle for blitting the animation.
> + /// 'position' is where the top left corner of the animation will end
> up,
> + /// 'source_rect' is the rectangle calculated by source_rectangle,
> + /// 'scale' is the zoom scale.
> + virtual Rectf
> + destination_rectangle(const Vector2f& position, const Rectf&
> source_rect, float scale) const = 0;
>
> /// The number of animation frames of this animation.
> virtual uint16_t nr_frames() const = 0;
--
https://code.launchpad.net/~widelands-dev/widelands/animation_scaling/+merge/310718
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/animation_scaling.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
