Code review comment for lp:~dpm/qreator/touch-scanner

Revision history for this message
David Planella (dpm) wrote :

On Sun, Jun 16, 2013 at 4:18 PM, Schwarzburg <
<email address hidden>> wrote:

> Review: Approve
>
> Hi David,
>
> First of all: Really great work!
>
> I played with it on the desktop and it worked quite well

Cool, thanks!

> (except for the camera..., now I have also pictures that are half bright,
> half dark :-) but thats another topic...).
>
>
Yeah, the Camera QML module (and perhaps also the desktop GStreamer
backend) is proving to be a bit of a pain :/

> I'm not surprised about the increase of performance with C++, and I think
> we should keep that in mind as advice for others and ourselves in the
> future: QML/JS for the interface, C++ for intensive tasks. On mobile
> devices, snappy applications are important!
>
>
Indeed. Still, I haven't quite given up on it, as JS does offer advantages
for development (if performance would be on par), as it avoids the need to
do cross-compilation (or at least compile for ARM on the device itself):
I've tried to downscale the image before sending it to the JS decoder, but
so far I've been unsuccessful:

https://code.launchpad.net/~dpm/qreator/touch-decoder-improvements

In any case, I've now created a project for the C++ plugin and I'm going to
get the daily package builds from there:

https://launchpad.net/qzxing

>
> I do have a few minor questions:
>
> - you wrote "qt5declarative-qzxing-plugin", but you meant
> "qtdeclarative5-qzxing-plugin", right?
>
>
You're right, fixed it on the description.

> - Did you remove the history tab since it is still empty, or did you
> change your mind about including it as a tab, or do you want it removed in
> general?
>
>
I still want history as a tab. If I removed any content there was in it, it
was unintentional, but I didn't recall it containing anything yet.

> - I guess that the debug tab is just there temporary? Do you want to
> remove it for every release but keep it there in-between releases?
>
>
Yeah, the debug tab is temporary. You might have seen the comment where I
tried to make it "hideable" via a property, but it turns out it is not
possible at the moment (
https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1190972).

Yes, until Qt Creator gains support for debugging code on devices it might
be worth keeping for in between releases, and comment it out for releases.

>
> As I said, great work!
>

Thanks for the review!

> --
> https://code.launchpad.net/~dpm/qreator/touch-scanner/+merge/169683
> You are the owner of lp:~dpm/qreator/touch-scanner.
>

« Back to merge proposal