[effects/blur] Clean up shader code
ClosedPublic

Authored by zzag on May 25 2018, 12:35 PM.

Details

Summary
  • Drop abstract BlurShader class
  • Delete evil "using namespace KWin"
  • Fix includes
  • Use smart pointers
  • Turn BlurShader into a QObject
  • Fix coding style
  • Add missing default cases
  • Use default member initialization
  • Delete methods that are used only once
  • Use more const
  • Use QRect::{top,right,bottom,left} methods in the setBlurRect method

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.
zzag created this revision.May 25 2018, 12:35 PM
Restricted Application added a project: KWin. · View Herald TranscriptMay 25 2018, 12:35 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.May 25 2018, 12:35 PM
zzag added a comment.May 25 2018, 12:35 PM

While I was working on this diff, I noticed several issues:

  • setBlurRect sets "incorrect" blur rect because of QRect::right() and QRect::bottom(). Maybe, use QRectF?
  • unbind() doesn't reset m_activeSampleType.
zzag edited the summary of this revision. (Show Details)May 25 2018, 12:39 PM
zzag updated this revision to Diff 35120.May 29 2018, 2:27 PM

Rebase.

zzag updated this revision to Diff 35124.May 29 2018, 3:41 PM

Split the diff into multiple commits:

  • Drop abstract BlurShader class
  • Delete evil "using namespace KWin"
  • Fix includes
  • Use smart pointers
  • Turn BlurShader into a QObject
  • Fix coding style
  • Add missing default cases
  • Use default member initialization
  • Delete methods that are used only once
  • Use more const
  • Use QRect::{top,right,bottom,left} methods in the setBlurRect method
zzag updated this revision to Diff 35125.May 29 2018, 3:43 PM

Fix previous update.

zzag edited the summary of this revision. (Show Details)May 29 2018, 3:45 PM
davidedmundson accepted this revision.Jun 18 2018, 10:14 AM
This revision is now accepted and ready to land.Jun 18 2018, 10:14 AM
zzag added a comment.Jun 18 2018, 11:31 AM

@davidedmundson Should I squash commits?

Should I squash commits?

Small atomic commits are good, but if each item in your bullet point list is an individual commit that's probably a bit extreme.

So personally I would squash them, but I don't mind either way.

zzag added a comment.Jun 18 2018, 12:53 PM

Small atomic commits are good, but if each item in your bullet point list is an individual commit that's probably a bit extreme.

Yeah, each bullet point is a commit (to make easier to review changes).

So personally I would squash them, but I don't mind either way.

OK, I will squash them.

This revision was automatically updated to reflect the committed changes.