Code review comment for lp:~dandrader/qtmir/surfaceItemFillMode

Revision history for this message
Loïc Molinari (loic.molinari) wrote :

Using polish() sounds like the right thing to do in order to correctly sync surface size and item size, no more temporary frame drawn with surface size different than item size. Emitting the item sizeChanged signal in the GUI thread right before the updatePaintNode() callback in the render thread doesn't imply 2 frames anymore when Unity requires a surface resize. It should also fix that [1].

Regarding the fill mode, I think we should expose the same enum names that the standard QtQuick Image uses with, for now, just the needed ones : Stretch and Pad. That would make things cleaner IMO, easier to understand for people used to that and would allow to add the PreserveAspectFit and PreserveAspectCrop (and maybe the alignment props) later if ever needed.

(Take my comments with a grain of salt because I'm not completely aware of the whole picture, but I'm taking the liberty of commenting on that because I've been working a bit on qtmir lately for the mutlibuffer stream thing and for the AA optimization which kinda touches the same code path ;) )

[1] https://code.launchpad.net/~dandrader/qtmir/revertRev415/+merge/278604

« Back to merge proposal