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
Branch
sebas/drmbackend-split
Lint
No Linters Available
Unit
No Unit Test Coverage
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
24

I don't see QPointer used

backends/drm/drm_output.h
23

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
23

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.