efl_canvas_animation_rotate: adjust API
ClosedPublic
Actions

Authored by bu5hm4n on Oct 10 2019, 6:47 AM.

Details

Reviewers

Jaehyun_Cho
zmike
segfaultxavi

Maniphest Tasks

T8288: Animation API

Commits

rEFLf47b9277453e: efl_canvas_animation_rotate: adjust API

Summary

in task T8288 we concluded that a few APIs need to be adjusted in order
to stabelize animation classes at some point. This also adds a new macro
to eina in order to create EINA_VECTOR2 values more easily.

ref T8288

Diff Detail

Repository

rEFL core/efl

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

bu5hm4n created this revision.Oct 10 2019, 6:47 AM

Herald added a project: efl. · View Herald TranscriptOct 10 2019, 6:47 AM

Herald added subscribers: committers, reviewers, cedric. · View Herald Transcript

bu5hm4n requested review of this revision.Oct 10 2019, 6:47 AM

Harbormaster completed remote builds in B13933: Diff 26023.Oct 10 2019, 6:47 AM

bu5hm4n mentioned this in T8288: Animation API.Oct 10 2019, 6:48 AM

I like this addition. I just prefer when API being added to Eina are done in two step, one patch for eina and another one rolling it in everywhere. For the current case, it is just a one line, so I won't oppose this patch landing, but I just wanted to let you know my preference for future reference.

src/lib/eina/eina_vector.h
35	Is our preferred doc cop @segfaultxavi happy here?

segfaultxavi requested changes to this revision.Oct 10 2019, 1:06 PM

segfaultxavi added inline comments.

src/lib/eina/eina_vector.h
35	NO, obviously. But I was unsure on how we document macros so I let it slip :)

This revision now requires changes to proceed.Oct 10 2019, 1:06 PM

bu5hm4n requested review of this revision.Oct 28 2019, 12:44 AM

bu5hm4n added inline comments.

src/lib/eina/eina_vector.h
35	None of these macros is documented in eina. I do not want to start a discussion here on how to document them. Futher more, its a Vector 2 with 2 fields, what would you think this is doing ? brewing some coffee ? :P

segfaultxavi resigned from this revision.Oct 28 2019, 3:27 AM

segfaultxavi added inline comments.

src/lib/eina/eina_vector.h
35	Hrmpffff mumble mumble mumble I am not happy but yeah, OK. But at some point ALL public-facing API (macros included) will have to be documented.

update

bu5hm4n added a child revision: D10558: efl_canvas_animation: be more explicit with errors.Oct 30 2019, 1:52 AM

Harbormaster completed remote builds in B14257: Diff 26522.Oct 30 2019, 1:54 AM

Seems fine, you can clean the nits up when you land it.

src/lib/evas/canvas/efl_canvas_animation_rotate.eo
22	What does that even mean tho?
30	`center_point` ?

This revision is now accepted and ready to land.Oct 30 2019, 5:34 AM

Seems ok to me.
Same comment with @zmike about "center_pointer".

Closed by commit rEFLf47b9277453e: efl_canvas_animation_rotate: adjust API (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>, committed by segfaultxavi). · Explain WhyNov 4 2019, 4:19 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			src/bin/elementary/test_efl_anim_group_parallel.c (4 lines)
M			src/bin/elementary/test_efl_anim_group_sequential.c (4 lines)
M			src/bin/elementary/test_efl_anim_rotate.c (12 lines)
M			src/lib/eina/eina_vector.h (1 line)
M			src/lib/evas/canvas/efl_canvas_animation_rotate.c (44 lines)
M			src/lib/evas/canvas/efl_canvas_animation_rotate.eo (8 lines)
M			src/lib/evas/canvas/efl_canvas_animation_rotate_private.h (9 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	26023	f3dedfe		Oct 10 2019, 6:47 AM	★	★
Diff 2	26522	f4019d3	update	Oct 30 2019, 1:44 AM	★	★
Diff 3	26660	eb371c9	rEFLf47b9277453edb1a3b4b68171d99f5b841e52e19	Nov 4 2019, 4:06 AM	★	★

Status	Author	Revision
Closed	segfaultxavi	D10565 docs: Polish Efl.Canvas.Animation_*
Closed	bu5hm4n	D10562 efl_canvas_animation_translate: move from x&y to container types
Closed	segfaultxavi	D10560 efl_canvas_animation: improve generally
Closed	bu5hm4n	D10559 efl_canvas_animation_scale: move from 2 doubles to vectors
Closed	bu5hm4n	D10558 efl_canvas_animation: be more explicit with errors
Closed	bu5hm4n	D10350 efl_canvas_animation_rotate: adjust API