Page MenuHomePhabricator

wl-drm: Refactor _drm2_randr_apply function
ClosedPublic

Authored by devilhorns on Mar 6 2019, 9:46 AM.

Details

Summary

This patch refactors _drm2_randr_apply inside the wl_drm module in
order to support multiple outputs and to fix issue of rotation not
working

ref T7690

Diff Detail

Repository
rE core/enlightenment
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
devilhorns created this revision.Mar 6 2019, 9:46 AM
devilhorns requested review of this revision.Mar 6 2019, 9:46 AM
cedric requested changes to this revision.Mar 6 2019, 10:34 AM
cedric added inline comments.
src/modules/wl_drm/e_mod_main.c
423

This line seems weird. If the symbol is not provided, then we should maybe have a wrapper that do a version check and then a dlsym and return a valid initialization in all case.

598–626

This super deep if could be inverted with something like :

if (!s->config.configured)
  {
    printf("RRR: unconfigured screen: %s\n", s->info.name);
    continue;
  }

This apply on the entire if body below.

678

My comment on previous use of version minimum. Do we need a dlsym wrapper here too?

683

What is the point of pushing new dead code?

This revision now requires changes to proceed.Mar 6 2019, 10:34 AM
devilhorns added inline comments.Mar 6 2019, 10:47 AM
src/modules/wl_drm/e_mod_main.c
423

the E_EFL_VERSION_MINIMUM is doing a version check... but we can't dlsym to a function that does not exist in older EFL versions... but yea I see what you mean in that it's going to make compile for E fail if it's an older version. I'll fix this up before it gets pushed.

598–626

Sure, I'll fix that up

678

the E_EFL_VERSION_MINIMUM is doing a version check... but we can't dlsym to a function that does not exist in older EFL versions... but yea I see what you mean in that it's going to make compile for E fail if it's an older version. I'll fix this up before it gets pushed.

683

This will be uncomented after the EFL freeze is over and I can push new ecore_drm2 clone functions to support this. Yea, I could remove all this code (only to re-add it later) if that would make you happy....

cedric added inline comments.Mar 6 2019, 10:49 AM
src/modules/wl_drm/e_mod_main.c
423

Yeah, you would do the dlsym only if the version minimum is high enough and return a default value if not available.

devilhorns updated this revision to Diff 20464.Mar 12 2019, 6:36 AM

Updated commit based on review comments. Now uses a proper dlsym to

link to new functions. Removed dead code commented out. Fixes super

deep if statement

devilhorns marked 9 inline comments as done.Mar 12 2019, 6:36 AM
devilhorns updated this revision to Diff 20483.Mar 12 2019, 7:55 AM
devilhorns added a project: enlightenment-git.

Fix comilation failures

devilhorns updated this revision to Diff 20509.Mar 13 2019, 7:35 AM

Fixed issue of e_drm2.x not properly linking for 1.22.0 added functions

This revision was not accepted when it landed; it landed in state Needs Review.Mar 22 2019, 10:01 AM
This revision was automatically updated to reflect the committed changes.