Differential D13246

[effects/blur] Check for blitting support
ClosedPublic
Actions

Authored by anemeth on May 31 2018, 7:03 PM.

Details

Reviewers
davidedmundson
Group Reviewers
KWin
Commits
R108:be3168b832fb: [effects/blur] Check for blitting support
R108:81f38abe6d6d: [effects/blur] Check for blitting support
Summary

In D12678 blur was changed to use blitFromFramebuffer() instead of glCopyTexSubImage2D()
Now it checks if the GPU supports it.

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.
anemeth created this revision.May 31 2018, 7:03 PM
Restricted Application added a project: KWin. · View Herald TranscriptMay 31 2018, 7:03 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
anemeth requested review of this revision.May 31 2018, 7:03 PM
anemeth edited the summary of this revision. (Show Details)May 31 2018, 7:06 PM
anemeth added reviewers: KWin, davidedmundson.
anemeth retitled this revision from Check for blitting support to [effects/blur] Check for blitting support.May 31 2018, 7:09 PM
davidedmundson accepted this revision.Jun 1 2018, 9:39 AM
This revision is now accepted and ready to land.Jun 1 2018, 9:39 AM
Closed by commit R108:81f38abe6d6d: [effects/blur] Check for blitting support (authored by anemeth). · Explain WhyJun 1 2018, 10:19 AM
This revision was automatically updated to reflect the committed changes.
zzag added a subscriber: zzag.Jun 1 2018, 10:38 AM

I'm a little bit too late, but...

effects/blur/blur.cpp
336

IMHO, length of this line is too big now.

const bool supported = effects->isOpenGLCompositing
    && GLRenderTarget::supported()
    && GLRenderTarget::blitSupported();

would be better.

anemeth added inline comments.Jun 1 2018, 10:52 AM
effects/blur/blur.cpp
336

You make a good point, but there are lines already longer than this in blur.cpp
Anyways you could create a revision that splits long lines into multiple ones.
The value of supported might be changed below so it can't be const (although scrolling through the file I counted multiple occurrences where const could be added, maybe another revision?)

Revision Contents
Changeset List

PathPackages
Meffects/blur/blur.cpp (2 lines)

Diff 35312

View Options

effects/blur/blur.cpp

Loading...