Split drm_backend.{h,cpp} into separate files
ClosedPublic

Authored by sebas on Mar 18 2016, 12:36 AM.

Details

Summary

This changes splits up the monster file containing different types used
in the DRM backend into separate files per class:

  • drm_backend.{h,cpp}
  • drm_buffer.{h,cpp}
  • drm_inputeventfilter.{h,cpp}
  • drm_output.{h,cpp}
  • drm_pointer.h

No actual code changes other than build fixes.

Clean up headers in the split files.

Test Plan

Builds with GBM enabled

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sebas updated this revision to Diff 2829.Mar 18 2016, 12:36 AM
sebas retitled this revision from to Split drm_backend.{h,cpp] into separate files.
sebas updated this object.
sebas edited the test plan for this revision. (Show Details)
sebas added a reviewer: graesslin.
Restricted Application added a project: Plasma. · View Herald TranscriptMar 18 2016, 12:36 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
graesslin edited edge metadata.Mar 18 2016, 6:48 AM

does it still make sense to prefix everything with "drm_"?

Thanks for doing the split!

backends/drm/drm_backend.cpp
49–50

please remove if no longer needed

backends/drm/drm_backend.h
54–56

why are the forward declarations removed? If that compiled before the split with forward declarations instead of includes that should still be possible

backends/drm/drm_buffer.cpp
21–22

Krazy would tell you to include own header first

25

I don't see this one used

26–28

I don't see this one used

backends/drm/drm_inputeventfilter.h
26

I don't see QPointer used her

27

I don't see any QSize in this file

backends/drm/drm_output.cpp
274–275

Please don't push your local TODO commits

597–599

similar

backends/drm/drm_pointer.h
24

no QPointer in this file, though I see a QScopedPointer

backends/drm/scene_qpainter_drm_backend.cpp
53–58

unrelated to the split

graesslin requested changes to this revision.Mar 21 2016, 10:57 AM
graesslin edited edge metadata.
This revision now requires changes to proceed.Mar 21 2016, 10:57 AM
sebas added a comment.Mar 21 2016, 1:03 PM

does it still make sense to prefix everything with "drm_"?

I would say so, at least in my hacking sessions, I have tons of files called output, backend, etc., so for me personally, it's actually very convenient.

sebas updated this revision to Diff 2876.Mar 21 2016, 1:33 PM
sebas edited edge metadata.
sebas marked 7 inline comments as done.
  • address review comments
  • remove comments that are only useful to sebas
graesslin requested changes to this revision.Mar 21 2016, 1:44 PM
graesslin edited edge metadata.

There are still a few unrelated changes included and some not-needed includes in header files

backends/drm/drm_buffer.h
25

I don't see QPointer used

backends/drm/drm_output.h
24

either forward declare (see below the forward declaration for DrmBackend) or include, not both :-) I assume that forward declaration should be sufficient here.

backends/drm/drm_pointer.h
24

QPointer not used in this file

backends/drm/scene_qpainter_drm_backend.cpp
54–58

still unrelated

This revision now requires changes to proceed.Mar 21 2016, 1:44 PM
sebas updated this revision to Diff 2877.Mar 21 2016, 1:55 PM
sebas edited edge metadata.
sebas marked 2 inline comments as done.
  • address review comments
  • remove comments that are only useful to sebas
  • more include cleanups
sebas updated this revision to Diff 2878.Mar 21 2016, 2:01 PM
sebas retitled this revision from Split drm_backend.{h,cpp] into separate files to Split drm_backend.{h,cpp} into separate files.
sebas edited edge metadata.
  • address review comments
  • remove comments that are only useful to sebas
  • more include cleanups
sebas updated this revision to Diff 2879.Mar 21 2016, 2:02 PM
  • address review comments
  • remove comments that are only useful to sebas
  • more include cleanups
  • Remove comments
graesslin accepted this revision.Mar 21 2016, 2:03 PM
graesslin edited edge metadata.
This revision is now accepted and ready to land.Mar 21 2016, 2:03 PM
This revision was automatically updated to reflect the committed changes.