romangg (Roman Gilg)
User

Projects

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Friday

  • Clear sailing ahead.

User Details

User Since
Apr 21 2016, 2:20 PM (173 w, 6 d)
Availability
Available

Recent Activity

Today

romangg added a comment to D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.

I always had tearing when watching fullscreen videos on my Nvidia setup, but with this patch I haven't noticed any tearing so far!

Wed, Aug 21, 9:19 AM · KWin
romangg updated the summary of D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
Wed, Aug 21, 9:09 AM · KWin
romangg updated the diff for D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
  • Perform compositing always from event loop
  • Remove unneeded sync, retrace block plumbing
  • Remove swap profiler code
Wed, Aug 21, 9:06 AM · KWin

Yesterday

romangg updated the summary of D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
Tue, Aug 20, 6:55 PM · KWin
romangg added a comment to D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.

I tested it now with Nvidia proprietary driver (v430, Neon unstable, GTX 950) and as expected the fallback timer is used since OML sync control and intel swap event are not available. This means there is no synchronization / delay minimization through delay timer in relation to the vblanks. There is of course the fallback timer providing a gap of ~16ms and the result looks still ok for me. Did not notice any tearing or freezes.

Tue, Aug 20, 6:15 PM · KWin
romangg updated the diff for D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
  • No need for setting __GL_MaxFramesAllowed
Tue, Aug 20, 6:08 PM · KWin
romangg updated the task description for T11423: Website Bof Akademy 2019.
Tue, Aug 20, 5:35 PM · Websites
romangg updated the diff for D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.

Rebase on master.

Tue, Aug 20, 5:14 PM · KWin
romangg updated the diff for D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
  • Simplify fallback timer
Tue, Aug 20, 4:20 PM · KWin
romangg committed R108:99d921707fc7: [platforms/x11/standalone] Fix stored refresh rate (authored by romangg).
[platforms/x11/standalone] Fix stored refresh rate
Tue, Aug 20, 12:03 PM
romangg closed D23265: [platforms/x11/standalone] Fix stored refresh rate.
Tue, Aug 20, 12:03 PM · KWin
romangg added a comment to D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.

@alexeymin Can you test again with the update? I just accidentally had the fallback timer disabled before. Now works fine for me without swap event.

Tue, Aug 20, 11:07 AM · KWin
romangg updated the diff for D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
  • Rebase on master
  • Use fallback timer without swap event, move present
Tue, Aug 20, 10:58 AM · KWin

Mon, Aug 19

romangg added inline comments to D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
Mon, Aug 19, 9:28 PM · KWin
romangg added a comment to D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.

With KWIN_USE_INTEL_SWAP_EVENT=1 all seems to work fine.

But with KWIN_USE_INTEL_SWAP_EVENT=0 or when unset, I see serious "lags" when dragging the window, it becomes very hard to drag window on screen, looks like some mouse movements are dropped(?) Resizing for example is still fine:

Mon, Aug 19, 9:14 PM · KWin
romangg added inline comments to D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
Mon, Aug 19, 8:34 PM · KWin
romangg added a comment to D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.

NVIDIA doesn't support the OML extensions. They can't be implemented efficiently on their hardware IIRC.
[...]

That's good to know. Thanks! I believe we can let in some of these extensions again without increasing the complexity too much as long as the SGI ones is ignored and we have no manual control of vsync. The complexity in the old code came mostly from that.

That would leave the user without a way to override the driver's default vsync setting.

Mon, Aug 19, 6:34 PM · KWin
romangg requested review of D23265: [platforms/x11/standalone] Fix stored refresh rate.
Mon, Aug 19, 2:31 PM · KWin

Sat, Aug 17

romangg added a comment to D23224: Fix capitalization of Nextcloud.

This means only change it in lines without [..].

Sat, Aug 17, 4:49 PM · Frameworks

Wed, Aug 14

romangg added a comment to D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.

NVIDIA doesn't support the OML extensions. They can't be implemented efficiently on their hardware IIRC.
[...]

Wed, Aug 14, 10:04 PM · KWin
romangg added a comment to D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.

What I really need additionally is testing it on an Nvidia setup.

I've briefly tested it on my NVidia GTX 550Ti with nvidia driver. Nothing appears to be horribly broken. Anything in particular I should look out for?

Wed, Aug 14, 4:04 PM · KWin

Tue, Aug 13

romangg added a revision to T11071: Rework compositing pipeline: D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
Tue, Aug 13, 4:37 PM · KWin
romangg added a task to D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing: T11071: Rework compositing pipeline.
Tue, Aug 13, 4:37 PM · KWin
romangg updated the summary of D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
Tue, Aug 13, 3:51 PM · KWin
romangg added a comment to D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.

Some observations:

Tue, Aug 13, 3:47 PM · KWin
romangg updated the diff for D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
  • Compositor swap events without timer
  • Paint delay timer
  • For analysis add ftrace marker from D23114
Tue, Aug 13, 3:40 PM · KWin

Mon, Aug 12

romangg updated the summary of D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
Mon, Aug 12, 6:08 PM · KWin
romangg updated the summary of D23114: [perf] Introduce ftrace marker.
Mon, Aug 12, 3:17 PM · KWin
romangg updated the summary of D23114: [perf] Introduce ftrace marker.
Mon, Aug 12, 3:12 PM · KWin
romangg requested review of D23114: [perf] Introduce ftrace marker.
Mon, Aug 12, 3:08 PM · KWin
romangg planned changes to D22973: docs: introduce commit message guideline.

If at all this will be embedded into a push on wider scope in all of KDE or at least Plasma and Frameworks, see https://mail.kde.org/pipermail/plasma-devel/2019-August/101494.html.

Mon, Aug 12, 3:06 PM · KWin
romangg updated the diff for D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.

Rebase on master

Mon, Aug 12, 2:51 PM · KWin
romangg committed R110:989df1eec1f4: Wayland: Rename output member variable (authored by romangg).
Wayland: Rename output member variable
Mon, Aug 12, 1:21 PM
romangg closed D23095: Wayland: Rename output member variable.
Mon, Aug 12, 1:21 PM · Plasma
romangg closed D23094: Wayland: Improve screen code style.
Mon, Aug 12, 1:19 PM · Plasma
romangg committed R110:c80a7ab40abc: Wayland: Improve screen code style (authored by romangg).
Wayland: Improve screen code style
Mon, Aug 12, 1:19 PM
romangg added a comment to D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.

I haven't yet tested it on AMD

I could test how it works on amdgpu (RX560), if you need help with that, when it's ready to test

Mon, Aug 12, 11:10 AM · KWin
romangg added a reviewer for D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing: fredrik.
Mon, Aug 12, 10:25 AM · KWin
romangg added a comment to D23102: Reduce duplicate code calculating popup position.

Maybe add a comment why the y value needs to be adjusted in this one special case.

Mon, Aug 12, 9:57 AM · KWin
romangg accepted D23102: Reduce duplicate code calculating popup position.

Your comments explain the new control flow just fine and why it is equivalent to the old one. You can add the const on push, no need to update the diff on Phab before.

Mon, Aug 12, 9:54 AM · KWin
romangg updated the diff for D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
  • Restore some functions old positions to limit the diff size
Mon, Aug 12, 1:13 AM · KWin
romangg updated the diff for D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
  • Remove unused variables
Mon, Aug 12, 1:09 AM · KWin
romangg updated the summary of D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
Mon, Aug 12, 1:06 AM · KWin
romangg requested review of D23105: WIP: [platforms/x11] Cleanup GLX backend, revise compositing.
Mon, Aug 12, 1:05 AM · KWin

Sun, Aug 11

romangg accepted D23082: Allow to set maximum volume < 100%.

Thanks for the vid. Also I feel betrayed now by my system.

Sun, Aug 11, 10:29 PM · Plasma
romangg added a comment to D23082: Allow to set maximum volume < 100%.

When you're at it could you take a look at why hardware keys are not working above 100%? Shouldn't be handled with a separate patch though.

Sun, Aug 11, 10:20 PM · Plasma
romangg added a comment to D23102: Reduce duplicate code calculating popup position.

Did you test the menu position in particular in edge cases?

Sun, Aug 11, 8:03 PM · KWin
romangg accepted D23101: Move common code out of if/else.
Sun, Aug 11, 7:58 PM · KWin
romangg accepted D23067: Clean up usage of m_client.
Sun, Aug 11, 7:04 PM · KWin
romangg accepted D23086: Be consistent about touch point id type: use qint32.

Great, thanks. Please make sure arc won't fail on this commit. As a tip: when I'm unsure about it I use arc land with the --holdoption. This way I can inspect afterwards if the commit message and everything is alright before pushing normally with git the resulting commit to the remote.

Sun, Aug 11, 6:38 PM · KWin
romangg accepted D23067: Clean up usage of m_client.
Sun, Aug 11, 6:31 PM · KWin
romangg added inline comments to D23067: Clean up usage of m_client.
Sun, Aug 11, 6:30 PM · KWin
romangg added a comment to D23098: Don't crash when X11Compositor tears down.

Are you doing this deliberately? I said on tear down it makes sense to not process the calls to updateCompositeBlocking...

Sun, Aug 11, 6:24 PM · KWin
romangg added a comment to D23098: Don't crash when X11Compositor tears down.

Replacing = 0 with {} in the header file is out of scope?

Sun, Aug 11, 6:21 PM · KWin
romangg added a comment to D23098: Don't crash when X11Compositor tears down.

It makes sense on tear down to not process any updateCompositeBlocking calls any more since compositing won't be enabled any more.

Sun, Aug 11, 6:17 PM · KWin
romangg added a comment to D23098: Don't crash when X11Compositor tears down.
In D23098#510410, @zzag wrote:

Thanks, makes sense. Then just add a default implementation in base class doing nothing? Or will it then still try to access the overriding subclass method?

It doesn't matter whether default implementation is provided. All updateCompositingBlocked() method calls will be dispatched to Compositor, not X11Compositor or WaylandCompositor.

Sun, Aug 11, 4:31 PM · KWin
romangg added a comment to D23098: Don't crash when X11Compositor tears down.
In D23098#510408, @zzag wrote:

How about just telling me?

Any call made to a virtual method in constructor/destructor of the base class won't go to a derived class because that method may access either uninitialized or destroyed resources.

Sun, Aug 11, 3:54 PM · KWin
romangg added a comment to D23098: Don't crash when X11Compositor tears down.

How about just telling me?

Sun, Aug 11, 3:42 PM · KWin
romangg added a comment to D23098: Don't crash when X11Compositor tears down.

Why does it work with the other destructor then? Destructors are called one after the other, right?

Sun, Aug 11, 3:32 PM · KWin
romangg added a comment to D23067: Clean up usage of m_client.

I hope this is now making the code slightly easier to read, without changing the functionallity, except for shuffling things around a tiny bit where we discussed. It also adds a few extra checks, to play safe with the smart pointers. These nullptr checks are extremely cheap.

Sun, Aug 11, 2:34 PM · KWin
romangg added inline comments to D23067: Clean up usage of m_client.
Sun, Aug 11, 2:32 PM · KWin
romangg added a comment to D23086: Be consistent about touch point id type: use qint32.

...
I though these were Qt touch points, interesting. In any case, I hope this is more complete than just casting in 16e904592ae4fe0a04ef61d42c9a114c62997c8e.
What is the best way to test these? I'll run my desktop with the change. I don't use the touch screen that often though.

Sun, Aug 11, 2:27 PM · KWin
romangg added a comment to D23086: Be consistent about touch point id type: use qint32.
In D23086#510314, @zzag wrote:

Someone "fixed" these warnings a while ago, but you reverted the fix.

I know. I reverted because it broke functionality. So there was no fix.

Sun, Aug 11, 2:08 PM · KWin
romangg requested review of D23095: Wayland: Rename output member variable.
Sun, Aug 11, 1:34 PM · Plasma
romangg requested review of D23094: Wayland: Improve screen code style.
Sun, Aug 11, 1:31 PM · Plasma
romangg accepted D23085: Make iterating over Layer enum simple.
Sun, Aug 11, 10:38 AM · KWin
romangg added a comment to D23086: Be consistent about touch point id type: use qint32.

This makes sense because libinput provides (non-negative) int32 ids/slots.

Sun, Aug 11, 10:33 AM · KWin
romangg accepted D23083: Be consistent in undefining macros.
Sun, Aug 11, 10:07 AM · KWin
romangg added a comment to D22973: docs: introduce commit message guideline.

To be honest this discussion about the necessity/preference of capitalizing a single letter doesn't lead to much and I don't want to continue it any more. I don't agree with the provided arguments and see no reason to deviate from the recommendation of Angular project and FWIW https://www.conventionalcommits.org. Either we take the current revision as it is or not at all.

Sun, Aug 11, 1:32 AM · KWin

Sat, Aug 10

romangg updated subscribers of D23069: Remove disabled TabGroup feature.

Wow, didn't thought it would be that much abandoned code. I'm definitely now for removing all of it. Even if we find the time to bring tabbed windows back at some point in the future it will probably be easier to just start again from scratch. And with this commit we can then still consult the old code for inspiration.

Sat, Aug 10, 11:03 PM · KWin
romangg added a comment to D22973: docs: introduce commit message guideline.
In D22973#509896, @zzag wrote:

I don't really follow this argument

The prefix is not part of a proper sentence. It has a bit special purpose, we just use a colon to separate it from the rest of subject line. Therefore, we can follow our own rules. Colon is just a stylistic choice I guess.

Sat, Aug 10, 8:54 PM · KWin
romangg added a comment to D22973: docs: introduce commit message guideline.
In D22973#509840, @zzag wrote:

The only comment I have is about capitalization. It's better to capitalize <subject> as the prefix serves a bit special purpose.

Sat, Aug 10, 6:48 PM · KWin
romangg updated the diff for D22973: docs: introduce commit message guideline.
  • Remove section about breaking changes
Sat, Aug 10, 6:42 PM · KWin
romangg accepted D23072: refactor: Minimize use of geom in Toplevel subclasses.

That's good. I hate this geom member variable without m_.

Sat, Aug 10, 3:28 PM · KWin
romangg updated subscribers of D22973: docs: introduce commit message guideline.

@yurchor: Thanks for the feedback. Corrected now most of them.

Sat, Aug 10, 3:23 PM · KWin
romangg updated the diff for D22973: docs: introduce commit message guideline.
  • Typos
Sat, Aug 10, 1:55 PM · KWin
romangg committed R110:137ec11fe2a6: Wayland: set KWayland outputs data in output class (authored by romangg).
Wayland: set KWayland outputs data in output class
Sat, Aug 10, 1:50 PM
romangg closed D23028: Wayland: set KWayland outputs data in output class.
Sat, Aug 10, 1:50 PM · Plasma
romangg committed R110:4d698bd16d41: Wayland: unfriend config and output classes (authored by romangg).
Wayland: unfriend config and output classes
Sat, Aug 10, 1:49 PM
romangg closed D23026: Wayland: unfriend config and output classes.
Sat, Aug 10, 1:49 PM · Plasma
romangg committed R110:6a1aef331f7e: Wayland: output device name as create function argument only (authored by romangg).
Wayland: output device name as create function argument only
Sat, Aug 10, 1:47 PM
romangg closed D23025: Wayland: output device name as create function argument only.
Sat, Aug 10, 1:47 PM · Plasma
romangg committed R110:e4bb0192df42: Wayland: save pointers in initializing list (authored by romangg).
Wayland: save pointers in initializing list
Sat, Aug 10, 1:45 PM
romangg closed D23024: Wayland: save pointers in initializing list.
Sat, Aug 10, 1:45 PM · Plasma
romangg added inline comments to D23067: Clean up usage of m_client.
Sat, Aug 10, 12:49 PM · KWin
romangg accepted D23071: Remove usage of QWeakPointer for QObject for AbstractThumbnailItem::m_parent.
Sat, Aug 10, 12:33 PM · KWin
romangg accepted D23070: Remove usage of QWeakPointer for QObject for thumbnails.
Sat, Aug 10, 12:32 PM · KWin
romangg added a comment to D23070: Remove usage of QWeakPointer for QObject for thumbnails.

Ok, I believe I got it now. The QWeakPointer constructer should get either passed a QSharedPointer or QWeakPointer as argument. But here we just pass the QObject EffectWindowImpl. Thanks for explaining.

Sat, Aug 10, 12:32 PM · KWin
romangg added a comment to D23069: Remove disabled TabGroup feature.

I just removed the obviously dead bits behind an if (false). If someone wants to bring it back, it's always in the git history. I don't care either way, but to me reducing lines of code is generally a good thing.

Sat, Aug 10, 11:55 AM · KWin
romangg added a comment to D23070: Remove usage of QWeakPointer for QObject for thumbnails.

See https://phabricator.kde.org/D23065

Using a QWeakPointer for QObjects works, but has been deprecated for around seven years. QPointer does the same cleanly (track life-time of QObjects). If you use .toStrongRef() on a QWeakPointer to a QObject without going through QSharedPointer you are in UB land.
In other words: this API is bad and will most likely be removed from Qt in the future.

Sat, Aug 10, 11:48 AM · KWin
romangg added a comment to D23070: Remove usage of QWeakPointer for QObject for thumbnails.

Sorry, but although you said that the change would be straight-forward can you explain it to me some more. Weak pointers are normally used in conjunction with shared ones. Is there a shared one somewhere?

Sat, Aug 10, 11:24 AM · KWin
romangg added a comment to D23071: Remove usage of QWeakPointer for QObject for AbstractThumbnailItem::m_parent.

Sorry, was just to write a commet on the other patch about the names of the commits and missing (?) part 1. See https://phabricator.kde.org/D23070#509746

Sat, Aug 10, 10:40 AM · KWin
romangg added a comment to D23070: Remove usage of QWeakPointer for QObject for thumbnails.

Where is part 1? Please don't name the commits "<many words> part x", "<many words> part y". Name explicitly each of them what the commit does. This one could be for example: "Remove thumbnails QWeakPointers".

Sat, Aug 10, 10:40 AM · KWin
romangg requested changes to D23071: Remove usage of QWeakPointer for QObject for AbstractThumbnailItem::m_parent.
Sat, Aug 10, 10:37 AM · KWin
romangg added inline comments to D23067: Clean up usage of m_client.
Sat, Aug 10, 10:32 AM · KWin
romangg accepted D23068: Fix ifdefs resulting in dead code.
Sat, Aug 10, 10:26 AM · KWin
romangg updated subscribers of D23069: Remove disabled TabGroup feature.

This is about window tabs? There was a patch preparing enabling them again at the beginning of the year by @graesslin. Don't know what happened afterwards. There is also m_switchToTabMenu. Is this not also about the same functionality?

Sat, Aug 10, 10:10 AM · KWin

Thu, Aug 8

romangg updated the task description for T11071: Rework compositing pipeline.
Thu, Aug 8, 2:56 PM · KWin
romangg added a task to D23032: Remove X11Compositor requires compositing check: T11071: Rework compositing pipeline.
Thu, Aug 8, 2:56 PM · KWin
romangg added a revision to T11071: Rework compositing pipeline: D23032: Remove X11Compositor requires compositing check.
Thu, Aug 8, 2:56 PM · KWin