Page MenuHomePhabricator

efl.file: improve api a bit
ClosedPublic

Authored by zmike on Feb 24 2019, 2:04 PM.

Details

Summary

the previous implementation/api had a number of issues:

  • "file" property contained both "file" and "key" values
    • also performed file loading operation
  • "load_error" property which was specific to image objects
  • no methods for controlling file loading/unloading

this patch attempts the following changes:

  • split "file" property into "file" and "key" properties
    • also remove "key" from existing "mmap" property
  • remove "load_error"
  • directly return error codes from operations
  • add "load" and "unload" methods for directly controlling load state
  • add implicit file loading if file/mmap is set during construction
  • rewrite all efl.file implementations to move file loading into load() method
  • rewrite all usage of efl.file api based on these changes
  • add C extension functions to mimic previous behavior

ref T7577

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.
zmike created this revision.Feb 24 2019, 2:04 PM
zmike requested review of this revision.Feb 24 2019, 2:04 PM

To be continued...

src/lib/edje/edje_load.c
793

You decided this is no longer an error? won't something break?

src/lib/efl/interfaces/efl_file.c
10–15

Mind adding some comments? The one I do not immediately understand is setting.

src/lib/efl/interfaces/efl_file.eo
42

Why isn't it a const(string) then?

62

Why isn't it a const(string) then?

100

Something broke here.

zmike added inline comments.Feb 25 2019, 10:53 AM
src/lib/edje/edje_load.c
793

This was returning 1 for success before. 0 is success when the type is Eina_Error.

src/lib/efl/interfaces/efl_file.eo
42

Dunno, I just used the existing docs.

100

My backspace key.

zmike updated this revision to Diff 19668.Feb 25 2019, 10:54 AM

add finalize implementions for some objects, improve docs

zmike updated this revision to Diff 19674.Feb 25 2019, 11:02 AM
zmike edited the summary of this revision. (Show Details)

rebase

segfaultxavi requested changes to this revision.Feb 26 2019, 4:53 AM

OK, this was a huge patch and I do not know enough internals to be 100% certain everything was done properly.
This is what I did, though:

  • make, make check && make examples all work
  • I checked lots of elementary_tests and all worked as expected.
  • I skimmed over lots of the code and didn't find anything obviously wrong. I only added one inline comment.
  • I like how the simpler uses of Efl.File are actually simpler now, since the NULL key is not needed anymore.
  • src/examples/elementary/efl_ui_list_example_1 does not have a sky background in elements 40 and 50 of the list, but that didn't happen before this patch, so...

So, all in all, I think this is good to go, once that inline comment is answered.

src/bin/elementary/test_ui_pager.c
83

Is the explicit invocation of efl_file_load required? Won't the file be automatically loaded by the finalizer?
I have seen this done in some places but not in others, why the inconsistency?

This revision now requires changes to proceed.Feb 26 2019, 4:53 AM
zmike added inline comments.Feb 26 2019, 12:02 PM
src/bin/elementary/test_ui_pager.c
83

I did it in some places and not others. There should be functionally no difference, as load is a no-op when called repeatedly.

zmike updated this revision to Diff 19702.Feb 26 2019, 1:15 PM
zmike edited the summary of this revision. (Show Details)

The src/examples/elementary/efl_ui_list_example_1 example now has the sky background, so great!

However, when running the Efl.Ui.Win (Bg part) elementary_test, I get this output on the console when clicking on the image with the green ivy leaves:

ERR<115790>:eo lib/eo/eo.c:569 _efl_object_call_resolve() in ../src/lib/efl/interfaces/efl_file.eo.c:84: func 'efl_file_loaded_get' (1142) could not be resolved for class 'Efl.Ui.Win'.

I cannot see what could be wrong there, the method is implemented.

zmike updated this revision to Diff 19720.Feb 27 2019, 6:16 AM

fix typo in win part implementation

The src/examples/elementary/efl_ui_list_example_1 example now has the sky background, so great!

However, when running the Efl.Ui.Win (Bg part) elementary_test, I get this output on the console when clicking on the image with the green ivy leaves:

ERR<115790>:eo lib/eo/eo.c:569 _efl_object_call_resolve() in ../src/lib/efl/interfaces/efl_file.eo.c:84: func 'efl_file_loaded_get' (1142) could not be resolved for class 'Efl.Ui.Win'.

I cannot see what could be wrong there, the method is implemented.

Typo. This is fixed. Waiting on @vitor.sousa to check cxx and then I think everything should be good to go?

segfaultxavi accepted this revision.Feb 27 2019, 6:35 AM

All questions answered. No further comments.

This revision is now accepted and ready to land.Feb 27 2019, 6:35 AM
zmike updated this revision to Diff 19729.Feb 27 2019, 9:17 AM

add @vitor.sousa patch to fix cxx

This revision was automatically updated to reflect the committed changes.