Re: Review Request 119465: Have separate texture hashes for each window

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119465/
---

(Updated July 28, 2014, 5:48 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-framework


Description
---

Apparently in nvidia we get corruptions when a texture created for a window is 
used in another one.
With this patch we tell the texture has changed when we move it from a window 
to another, so it's re-created and we keep textures for all windows separately. 
This way we ensure they don't mix.


Diffs
-

  src/declarativeimports/core/framesvgitem.h 0b39c70 
  src/declarativeimports/core/framesvgitem.cpp ebac29f 

Diff: https://git.reviewboard.kde.org/r/119465/diff/


Testing
---

Still works here, I hope Marco can confirm it fixes the problem.


Thanks,

Aleix Pol Gonzalez

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119465: Have separate texture hashes for each window

[David Edmundson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119465/#review63313
---



src/declarativeimports/core/framesvgitem.cpp


This must have been already happening; otherwise it would have been 
exploding in 5.0

QQuickItem.cpp:2687 resets the paintNode to 0.


- David Edmundson


On July 25, 2014, 2:28 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119465/
> ---
> 
> (Updated July 25, 2014, 2:28 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Apparently in nvidia we get corruptions when a texture created for a window 
> is used in another one.
> With this patch we tell the texture has changed when we move it from a window 
> to another, so it's re-created and we keep textures for all windows 
> separately. This way we ensure they don't mix.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.h 0b39c70 
>   src/declarativeimports/core/framesvgitem.cpp ebac29f 
> 
> Diff: https://git.reviewboard.kde.org/r/119465/diff/
> 
> 
> Testing
> ---
> 
> Still works here, I hope Marco can confirm it fixes the problem.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119465: Have separate texture hashes for each window

[Aleix Pol Gonzalez]

> On jul. 25, 2014, 3:43 p.m., Martin Gräßlin wrote:
> > The problem is not the different window, but probably the different OpenGL 
> > context. QtQuick uses with the threaded renderer an OpenGL context per 
> > QWindow. Marco could confirm that by enforcing the main thread renderer and 
> > verify that this also fixes the problem. If that's the case I'd rather go 
> > for verifying that it's the correct OpenGL context than the QWindow.
> 
> Marco Martin wrote:
> yep, with export QSG_RENDER_LOOP=basic seems rendering is correct
> 
> David Edmundson wrote:
> I don't think comparing contexts is possible here.
> Qt has a wrapper round the OpenGL context, QOpenGLContext.
> 
> You have one of those per window, even if underneath there is just one 
> OpenGL context. It's possible to get the "shareContext" but according to the 
> docs that can return 0 on some platforms at which point we're playing 
> dangerously.

Therefore it's correct that the patch is too conservative, no?


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119465/#review63166
---


On jul. 25, 2014, 2:28 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119465/
> ---
> 
> (Updated jul. 25, 2014, 2:28 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Apparently in nvidia we get corruptions when a texture created for a window 
> is used in another one.
> With this patch we tell the texture has changed when we move it from a window 
> to another, so it's re-created and we keep textures for all windows 
> separately. This way we ensure they don't mix.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.h 0b39c70 
>   src/declarativeimports/core/framesvgitem.cpp ebac29f 
> 
> Diff: https://git.reviewboard.kde.org/r/119465/diff/
> 
> 
> Testing
> ---
> 
> Still works here, I hope Marco can confirm it fixes the problem.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119465: Have separate texture hashes for each window

[Aleix Pol Gonzalez]

> On jul. 25, 2014, 4:36 p.m., Marco Martin wrote:
> > src/declarativeimports/core/framesvgitem.cpp, line 49
> > 
> >
> > when a qquickwindow gets deleted.. does it delete all the QSGTextures 
> > created with createTextureFromImage?
> 
> David Edmundson wrote:
> No. Ownership is always transferred to the recipient. (and this is using 
> QSharedPointer to then manage them)

The texture will get deleted when the node is. When we change window the 
texture gets destroyed (or de-referenced, since it's a qsharedpointer)


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119465/#review63172
---


On jul. 25, 2014, 2:28 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119465/
> ---
> 
> (Updated jul. 25, 2014, 2:28 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Apparently in nvidia we get corruptions when a texture created for a window 
> is used in another one.
> With this patch we tell the texture has changed when we move it from a window 
> to another, so it's re-created and we keep textures for all windows 
> separately. This way we ensure they don't mix.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.h 0b39c70 
>   src/declarativeimports/core/framesvgitem.cpp ebac29f 
> 
> Diff: https://git.reviewboard.kde.org/r/119465/diff/
> 
> 
> Testing
> ---
> 
> Still works here, I hope Marco can confirm it fixes the problem.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119465: Have separate texture hashes for each window

[David Edmundson]

> On July 25, 2014, 3:43 p.m., Martin Gräßlin wrote:
> > The problem is not the different window, but probably the different OpenGL 
> > context. QtQuick uses with the threaded renderer an OpenGL context per 
> > QWindow. Marco could confirm that by enforcing the main thread renderer and 
> > verify that this also fixes the problem. If that's the case I'd rather go 
> > for verifying that it's the correct OpenGL context than the QWindow.
> 
> Marco Martin wrote:
> yep, with export QSG_RENDER_LOOP=basic seems rendering is correct

I don't think comparing contexts is possible here.
Qt has a wrapper round the OpenGL context, QOpenGLContext.

You have one of those per window, even if underneath there is just one OpenGL 
context. It's possible to get the "shareContext" but according to the docs that 
can return 0 on some platforms at which point we're playing dangerously.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119465/#review63166
---


On July 25, 2014, 2:28 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119465/
> ---
> 
> (Updated July 25, 2014, 2:28 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Apparently in nvidia we get corruptions when a texture created for a window 
> is used in another one.
> With this patch we tell the texture has changed when we move it from a window 
> to another, so it's re-created and we keep textures for all windows 
> separately. This way we ensure they don't mix.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.h 0b39c70 
>   src/declarativeimports/core/framesvgitem.cpp ebac29f 
> 
> Diff: https://git.reviewboard.kde.org/r/119465/diff/
> 
> 
> Testing
> ---
> 
> Still works here, I hope Marco can confirm it fixes the problem.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119465: Have separate texture hashes for each window

[David Edmundson]

> On July 25, 2014, 4:36 p.m., Marco Martin wrote:
> > src/declarativeimports/core/framesvgitem.cpp, line 49
> > 
> >
> > when a qquickwindow gets deleted.. does it delete all the QSGTextures 
> > created with createTextureFromImage?

No. Ownership is always transferred to the recipient. (and this is using 
QSharedPointer to then manage them)


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119465/#review63172
---


On July 25, 2014, 2:28 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119465/
> ---
> 
> (Updated July 25, 2014, 2:28 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Apparently in nvidia we get corruptions when a texture created for a window 
> is used in another one.
> With this patch we tell the texture has changed when we move it from a window 
> to another, so it's re-created and we keep textures for all windows 
> separately. This way we ensure they don't mix.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.h 0b39c70 
>   src/declarativeimports/core/framesvgitem.cpp ebac29f 
> 
> Diff: https://git.reviewboard.kde.org/r/119465/diff/
> 
> 
> Testing
> ---
> 
> Still works here, I hope Marco can confirm it fixes the problem.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119465: Have separate texture hashes for each window

[Marco Martin]

> On July 25, 2014, 3:43 p.m., Martin Gräßlin wrote:
> > The problem is not the different window, but probably the different OpenGL 
> > context. QtQuick uses with the threaded renderer an OpenGL context per 
> > QWindow. Marco could confirm that by enforcing the main thread renderer and 
> > verify that this also fixes the problem. If that's the case I'd rather go 
> > for verifying that it's the correct OpenGL context than the QWindow.

yep, with export QSG_RENDER_LOOP=basic seems rendering is correct


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119465/#review63166
---


On July 25, 2014, 2:28 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119465/
> ---
> 
> (Updated July 25, 2014, 2:28 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Apparently in nvidia we get corruptions when a texture created for a window 
> is used in another one.
> With this patch we tell the texture has changed when we move it from a window 
> to another, so it's re-created and we keep textures for all windows 
> separately. This way we ensure they don't mix.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.h 0b39c70 
>   src/declarativeimports/core/framesvgitem.cpp ebac29f 
> 
> Diff: https://git.reviewboard.kde.org/r/119465/diff/
> 
> 
> Testing
> ---
> 
> Still works here, I hope Marco can confirm it fixes the problem.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119465: Have separate texture hashes for each window

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119465/#review63172
---



src/declarativeimports/core/framesvgitem.cpp


when a qquickwindow gets deleted.. does it delete all the QSGTextures 
created with createTextureFromImage?


- Marco Martin


On July 25, 2014, 2:28 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119465/
> ---
> 
> (Updated July 25, 2014, 2:28 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Apparently in nvidia we get corruptions when a texture created for a window 
> is used in another one.
> With this patch we tell the texture has changed when we move it from a window 
> to another, so it's re-created and we keep textures for all windows 
> separately. This way we ensure they don't mix.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.h 0b39c70 
>   src/declarativeimports/core/framesvgitem.cpp ebac29f 
> 
> Diff: https://git.reviewboard.kde.org/r/119465/diff/
> 
> 
> Testing
> ---
> 
> Still works here, I hope Marco can confirm it fixes the problem.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119465: Have separate texture hashes for each window

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119465/#review63166
---


The problem is not the different window, but probably the different OpenGL 
context. QtQuick uses with the threaded renderer an OpenGL context per QWindow. 
Marco could confirm that by enforcing the main thread renderer and verify that 
this also fixes the problem. If that's the case I'd rather go for verifying 
that it's the correct OpenGL context than the QWindow.

- Martin Gräßlin


On July 25, 2014, 4:28 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119465/
> ---
> 
> (Updated July 25, 2014, 4:28 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Apparently in nvidia we get corruptions when a texture created for a window 
> is used in another one.
> With this patch we tell the texture has changed when we move it from a window 
> to another, so it's re-created and we keep textures for all windows 
> separately. This way we ensure they don't mix.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.h 0b39c70 
>   src/declarativeimports/core/framesvgitem.cpp ebac29f 
> 
> Diff: https://git.reviewboard.kde.org/r/119465/diff/
> 
> 
> Testing
> ---
> 
> Still works here, I hope Marco can confirm it fixes the problem.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119465: Have separate texture hashes for each window

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119465/#review63164
---

Ship it!


on this system it does indeed fix the rendering issues.

can anyone else with an nvidia system (or other kinds of drivers) test as well?

- Marco Martin


On July 25, 2014, 2:28 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119465/
> ---
> 
> (Updated July 25, 2014, 2:28 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Apparently in nvidia we get corruptions when a texture created for a window 
> is used in another one.
> With this patch we tell the texture has changed when we move it from a window 
> to another, so it's re-created and we keep textures for all windows 
> separately. This way we ensure they don't mix.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.h 0b39c70 
>   src/declarativeimports/core/framesvgitem.cpp ebac29f 
> 
> Diff: https://git.reviewboard.kde.org/r/119465/diff/
> 
> 
> Testing
> ---
> 
> Still works here, I hope Marco can confirm it fixes the problem.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 119465: Have separate texture hashes for each window

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119465/
---

Review request for Plasma.


Repository: plasma-framework


Description
---

Apparently in nvidia we get corruptions when a texture created for a window is 
used in another one.
With this patch we tell the texture has changed when we move it from a window 
to another, so it's re-created and we keep textures for all windows separately. 
This way we ensure they don't mix.


Diffs
-

  src/declarativeimports/core/framesvgitem.h 0b39c70 
  src/declarativeimports/core/framesvgitem.cpp ebac29f 

Diff: https://git.reviewboard.kde.org/r/119465/diff/


Testing
---

Still works here, I hope Marco can confirm it fixes the problem.


Thanks,

Aleix Pol Gonzalez

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel