This revision was automatically updated to reflect the committed changes.
Kanedias marked 2 inline comments as done.
Closed by commit R108:247ef43f683b: Implement software cursor in OpenGL backend
(authored by Kanedias).
REPOSITORY
R108 KWin
CHANGES SINCE LAST UPDATE
graesslin accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R108 KWin
REVISION DETAIL
https://phabricator.kde.org/D6186
To: Kanedias, graesslin, davidedmundson
Cc: aacid, luebking, plasma-devel, kwin, #kwin, ZrenBot, progwolff, lesliezhai,
ali-mohamed,
graesslin added inline comments.
INLINE COMMENTS
> Kanedias wrote in scene_opengl.h:141
> Should we heal other methods suffering from this? I can prepare such patch.
No, we don't change the existing code. Only new code gets override.
REPOSITORY
R108 KWin
REVISION DETAIL
Kanedias marked 2 inline comments as done.
Kanedias added inline comments.
INLINE COMMENTS
> graesslin wrote in scene_opengl.cpp:682-685
> Instead of adding an empty method you could make it pure virtual
That moment when I noticed we have no SceneOpenGL instantiations. Tx!
> graesslin wrote in
Kanedias updated this revision to Diff 15860.
Kanedias marked an inline comment as done.
Kanedias added a comment.
Review comments fixes
REPOSITORY
R108 KWin
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D6186?vs=1=15860
REVISION DETAIL
https://phabricator.kde.org/D6186
graesslin requested changes to this revision.
graesslin added a comment.
This revision now requires changes to proceed.
Looks good to me. Just a minor comment regarding where to put the new virtual
method. Setting to reject changes as requested by Albert :-)
INLINE COMMENTS
>
aacid added a comment.
If this indeed needs fixing, can someone please mark it as "request changes"
so it doesn't show up in the list of "patches that have been accepted but have
not been commited"?
REPOSITORY
R108 KWin
REVISION DETAIL
https://phabricator.kde.org/D6186
To: Kanedias,
luebking added inline comments.
INLINE COMMENTS
> Kanedias wrote in scene_opengl.cpp:713
> How come! There's a condition on !m_cursorTexture for that, no? Or is
> Cursor::cursorChanged itself called on every paint?
Sorry, I missed that the "if (!m_cursorTexture) {" condition still covers these
Kanedias added inline comments.
INLINE COMMENTS
> luebking wrote in scene_opengl.cpp:713
> You still update the cursor texture with every paint() - there should be no
> need to hook this signal.
> Actually, the patch got much worse:
>
> a) you're fetching the image and reset the texture with
luebking added inline comments.
INLINE COMMENTS
> scene_opengl.cpp:713
> +// handle shape update on case cursor image changed
> +connect(Cursor::self(), ::cursorChanged, this,
> updateCursorTexture);
> +}
You still update the cursor texture with every paint() - there should
Kanedias marked 3 inline comments as done.
Kanedias added a comment.
Fixed lazy-init
REPOSITORY
R108 KWin
REVISION DETAIL
https://phabricator.kde.org/D6186
To: Kanedias, graesslin, davidedmundson
Cc: luebking, plasma-devel, kwin, #kwin, ZrenBot, spstarr, progwolff,
lesliezhai,
Kanedias updated this revision to Diff 1.
Kanedias added a comment.
Whoops, wrong patch
REPOSITORY
R108 KWin
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D6186?vs=15554=1
REVISION DETAIL
https://phabricator.kde.org/D6186
AFFECTED FILES
scene_opengl.cpp
Kanedias updated this revision to Diff 15554.
Kanedias added a comment.
Fixes for lazy-init (thanks T.L.)
REPOSITORY
R108 KWin
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D6186?vs=15397=15554
REVISION DETAIL
https://phabricator.kde.org/D6186
AFFECTED FILES
luebking added inline comments.
INLINE COMMENTS
> Kanedias wrote in scene_opengl.cpp:734
> You mean, to check whether BLEND was enabled prior to calling this func?
You might have to - and in addition possibly reset the actual blend function
from before. Please wait for Martins comment on this,
Kanedias added inline comments.
INLINE COMMENTS
> luebking wrote in scene_opengl.cpp:699
> The entire head should probably be in some init function, not in every paint
> call.
> If you go for a lazy init, you should seekt to prevent double connects (but I
> don't know whether
luebking added inline comments.
INLINE COMMENTS
> scene_opengl.cpp:699
> +// don't paint if no image for cursor is set
> +const QImage img = kwinApp()->platform()->softwareCursor();
> +if (img.isNull()) {
The entire head should probably be in some init function, not in every paint
Kanedias marked an inline comment as done.
Kanedias added a comment.
@graesslin , are you here? Can you look at this submission please if you have
time?
REPOSITORY
R108 KWin
REVISION DETAIL
https://phabricator.kde.org/D6186
To: Kanedias, graesslin, davidedmundson
Cc: plasma-devel,
davidedmundson accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R108 KWin
REVISION DETAIL
https://phabricator.kde.org/D6186
To: Kanedias, graesslin, davidedmundson
Cc: plasma-devel, kwin, #kwin, ZrenBot, spstarr, progwolff, lesliezhai,
ali-mohamed,
Kanedias marked an inline comment as done.
Kanedias added a comment.
I'm still unsure about cursor damage region (digging through cursor-related
signals now)
Cursor itself works fine, tested on kms Mesa 17.1.2 amdgpu (and in KRfb
session)
INLINE COMMENTS
> davidedmundson wrote in
Kanedias updated this revision to Diff 15397.
Kanedias added a comment.
Whoops... :)
REPOSITORY
R108 KWin
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D6186?vs=15396=15397
REVISION DETAIL
https://phabricator.kde.org/D6186
AFFECTED FILES
scene_opengl.cpp
scene_opengl.h
Kanedias updated this revision to Diff 15396.
Kanedias added a comment.
- Made cursor texture local var with lazy-init
- Made changes to support cursor shape update
- Added cursor hotspot handling
- Removed redundant vbo calls
REPOSITORY
R108 KWin
CHANGES SINCE LAST UPDATE
davidedmundson added a comment.
> Is this the correct way of doing such stuff?
Pretty much.
Seems a sensible patch, screen recording aside we could be using a GL on a
different backend that can't do planes.
The obvious optimisation is that we don't need to make the new cursor
Kanedias added a comment.
Hi Martin, David.
This is my first change in OpenGL-related stuff, where I don't yet have firm
ground.
I've tested this change on DRM session in my vt2 and it works, but I have
some concerns,
- Is this the correct way of doing such stuff?
- Should
Kanedias added dependencies: D1230: GBM remote access support for KWin, D1231:
Add Remote Access interface to KWayland.
REPOSITORY
R108 KWin
REVISION DETAIL
https://phabricator.kde.org/D6186
To: Kanedias, graesslin, davidedmundson
Cc: plasma-devel, kwin, #kwin, ZrenBot, spstarr, progwolff,
Kanedias created this revision.
Kanedias added a project: Plasma on Wayland.
Restricted Application added subscribers: KWin, kwin, plasma-devel.
Restricted Application added a project: KWin.
REVISION SUMMARY
Implement software cursor in OpenGL backend
This change is
25 matches
Mail list logo
