- 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
Details
Details
- Reviewers
davidedmundson - Group Reviewers
KWin - Commits
- R108:168109f3bb36: [effects/blur] Clean up shader code
Diff Detail
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.
Comment Actions
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.
Comment Actions
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
Comment Actions
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.
Comment Actions
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.