Page MenuHomePhabricator

Evas textblock: Apply scale factor to <linesize>, <linegap> formats
ClosedPublic

Authored by id213sin on Feb 15 2016, 9:15 PM.

Details

Summary

Font size is scaled according to scale factor.
The linesize, linegap formats also have to be scaled properly.
@fix

Test Plan

Test cases are included.
Run "make check"

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.
id213sin updated this revision to Diff 8449.Feb 15 2016, 9:15 PM
id213sin retitled this revision from to Evas textblock: Apply scale factor to <linesize>, <linegap> formats.
id213sin updated this object.
id213sin edited the test plan for this revision. (Show Details)
id213sin added reviewers: tasn, herdsman, woohyun.
tasn accepted this revision.Feb 16 2016, 3:20 AM
tasn edited edge metadata.

Looks good to me. If everything compiles without warnings and tests fail before this and pass now, I'm happy.

@herdsman is properly reviewing, so he'll check that. Sweet. :)

This revision is now accepted and ready to land.Feb 16 2016, 3:20 AM

@tasn
Thank you!
In my test, it does not have compile error or warning.
And the test cases are also failed without the changes on Evas Textblock.

tasn added a comment.Feb 16 2016, 5:19 AM

Sweet. I know @herdsman had a few minor comments, I wonder why he hasn't posted them yet. Will chase him now.

herdsman requested changes to this revision.Feb 16 2016, 5:32 AM
herdsman edited edge metadata.

I had done some testing to see the result and the pixels values. All looks good.

The castings in the patch are the only thing I would like to change here. Please see my inline comments.
Thanks for the hard work.

src/lib/evas/canvas/evas_object_textblock.c
2719–2720

The castings here have no special meaning.

int scaled_linesize = fmt->linesize * obj->cur->scale

This looks cleaner.

2722–2723

I see this was in the original code, and I wasn't going to ask for changes if it weren't for the other inline comment, but just keep in mind that the extra surrounding parentheses are not required.

2731

Same as the above inline comment.

This revision now requires changes to proceed.Feb 16 2016, 5:32 AM
id213sin updated this revision to Diff 8472.Feb 16 2016, 6:26 PM
id213sin edited edge metadata.

Remove explicit type casting and extra surrounding parentheses.

herdsman requested changes to this revision.Feb 17 2016, 7:01 AM
herdsman edited edge metadata.

You are shadowing a local variable in the test suite changes.
I don't want to mess with your patch, so I'm leaving this little thing to you.
Sorry about that.

This revision now requires changes to proceed.Feb 17 2016, 7:01 AM

Sorry, I missed that. :(
I will just move every declarations of Evas_Coord variables to the top of the function.

id213sin updated this revision to Diff 8495.Feb 17 2016, 6:32 PM
id213sin edited edge metadata.

Remove shadowing a local varaiable.

tasn added a comment.Feb 18 2016, 7:57 AM

Looks fine with me. @herdsman, is this ready to go in?

Ping!
I have to merge this patch to Tizen soon.
Could you land this patch if there is no problem? :)

jpeg added a comment.Jun 7 2016, 12:38 AM

@id213sin did you merge this in tizen?
@herdsman please merge this patch upstream if it's ok

actually i'll do it.

This revision was automatically updated to reflect the committed changes.