Unity 2D Wrong Colours for icons on PowerPC

Thanks for the report pauljwells. Could you please attach a screenshot?

Screenshots attached...

Hi Oliver

I attached a couple of screenshots to the bug report

Paul

On 13/04/11 09:26, Olivier Tilloy wrote:
> Thanks for the report pauljwells. Could you please attach a screenshot?
>
> ** Changed in: unity-2d (Ubuntu)
> Status: New => Incomplete
>

> I attached a couple of screenshots to the bug report

Thanks for the screenshots Paul! That looks rather bad indeed… yet it runs on PowerPC, and that is cool :)

It is very cool :-)

On 15/04/11 09:37, Olivier Tilloy wrote:
>> I attached a couple of screenshots to the bug report
> Thanks for the screenshots Paul! That looks rather bad indeed… yet it
> runs on PowerPC, and that is cool :)
>

From my limited previous experience on vaguely similar problems, I think this is a problem with the colour depth of the icons vs the capability of the video driver. I have an nvidea geforce 6600LE and the driver is nouveau. Since ubuntu ditched the xorg.conf file I don't know how to play with any of these things anymore.

I tried the nv driver, it gave me no display at all! Had to chroot to reinstall nouveau.
Also tried the libgl1-mesa-dri-experimental driver, no change
Tried different theme, the icons change but the colours remain 'weird'

I'm not familiar with the code for unity - if anyone who is could suggest some other things for me to try I'd be more than willing...

Icons using the default background colour look better than ones using the calculated colour - see screenshot. Is there a way of disabling the background colour? I saw in the qml files that there was a bool value to set this, but I could not find where the setting actually gets done. Any help much appreciated...

More guesswork...

This may not be a bug in unity-2d at all. I noticed that when running the 'matrix' screensaver on the ppc, the characters appear purple, rather than green like on my PC. I suspect that there is something more fundamental with the way colours are mapped for accelerated displays (??) The regular desktop colours look fine, the screensaver is running in OGL (software, hardware doesn't work ppc/nvidia)

Just for the record, these kind of problems with colors "changed" are usually the sign of wrong endianness (i386 and amd64 are little-endian, while powerpc is big-endian).

Regards, Rogério Brito.

Ah, big/little endian problem :-(
So it's not a Unity bug at all - is it a driver bug? Nouveau driver?

Hi, Paul.

Well, quite probably. It seems that the new gallium 3d drivers may have endianness issues at the moment. At least they seem to do with the ATI drivers.

I do plan on getting involved in the release of the next versions of Ubuntu for powerpc, now that the dust settles. In particular, I would like to get GRUB 2 to be the default bootloader for Macs. I have some proof of concept stuff at:

http://rb.doesntexist.org/blog/posts/running_grub2_on_powerpc_macs/

Of course, I do plan on working on other stuff as well.

Regards,
Rogério Brito.

Actually, I think there are two bugs here... It looks like QT has an endian problem (used in unity-2d) and nouveau has one (used in 3d - although I can only use software dri on my machine)

@Rogerio, very interesting work on grub! If I can be of any help with this or other issues I am keen to get more involved in development. I can just about write a python script or a simple C/C++ program...

Further digging reveals it might still be fixable within Unity-2D
Maybe in iconimageprovider.cpp ??
It looks like for powerpc architecture there needs to be a kludge to fix the QT behaviour/Apple behaviour mismatch to manually swap the byte order of the colours.

Fixing this is, frankly, beyond my current skill level... I hope to bring it to the attention of someone more proficient!

From http://permalink.gmane.org/gmane.comp.lib.qt.general/29931

> > Also pay attention to byte-ordering. QImage::Format_RGB32 and
> > Format_ARGB32 assume that the 32-bit pixel is in host endian. That is,
> > on a big-endian machine like PowerPC, the bytes in the buffer are in
> > ARGBARG... sequence while on a little-endian one like x86, they are
> > GBRAGBRA...
> >
> > >>From my (painful) experience dealing with Apple, they mostly assume
> > ARGBARGB sequence regardless of host endian. Unfortunately, Qt does not
> > provide an option to use reverse byte order image buffer:
> > <http://bugreports.qt.nokia.com/browse/QTBUG-5200>

http://<email address hidden>/2010-04/00352/Re-%28Qt-interest%29-Quicktime-C--gt-QImage.html

looks like it has the logic needed.

I've written a kludge that gives me the right colours! :-)

It needs to be wrapped in something to test for powerpc architecture, which I haven't yet learned how to do and to be packaged as a deb, likewise...

I'm pleased to have sorted out the logic though

Here's the kludged code - it's my first ever attempt at a patch

I'm now really hoping that a proper dev can do something with this. (I'm hinting at Olivier Tilloy here...)

My kludge doesn't seem to fix the icons in the dash (I think it's called) and I can't see where in the code the manipulation of these images takes place. Help!?

Awesome Paul, that looks very promising!
I will for sure have a look at your code, even though I don’t have PowerPC at hand to test.

Well, I have one right here ;-)

My code isn't pretty, but it works (I don't write a lot of C)

Like I posted, the dash and lenses still need the fix, but I'm hoping the same code could be slotted in, once I find where that is!

If I can do any testing let me know

Aha! Ignore #19. I just did a proper installation (creating debs rather than just running cmake directly) and the dash icons all work properly too! I now have a beautiful unity-2d interface on my G5 and am very happy. The underlying bug is really in Qt4, but I guess that's not going to get fixed.

@Olivier - How difficult will it be to write architecture-dependent code?

Great work Paul! As I mentioned earlier, I don’t have the hardware to test, but your patch looks sane to me. I have a couple of remarks:

1) To make this architecture-dependent, enclose your kludge in the following preprocessor directives:

    #if Q_BYTE_ORDER == Q_BIG_ENDIAN
    …
    #endif

2) The logic that inverts the components of each pixel is correct, but according to the documentation, QImage::pixel(…) and QImage::setPixel(…) are expensive. The documentation recommends to use QImage::scanLine(…) instead. I’ll see if I can rewrite your code with that in mind to make it more efficient.

I'm not surprised to hear that my code is inefficient, I was just pleased to get the byte-order correct and to prove the concept so at least that doesn't have to done again. I work in Python mostly and picked the most 'Pythonesque-looking' methods!

Thanks for the info on the preprocessor directives, that's very interesting for future reference.

If you want me to test any new code on the G5 be sure to let me know.

Looking at scanLine I wonder if we can't do something even lower-level. We don't actually need to use the QRgb, qRed, qBlue etc values - all we need to do is take each block of four bytes from the address given by scanLine and reverse their order. That should be the fastest way I would think?

I've got to take the dog for his walk right now but I'll have a go in an hour or so ;)

@Paul: I reworked a bit your code, it should hopefully be more efficient now for the same end result, you can check it out at lp:~osomon/unity-2d/bigendian. Testing and feedback welcome!

@Olivier: I built your code and I can confirm it works properly on my ppc G5.

Interesting to see how you directly manipulate the pixels using pointers; I have a lot to learn!

This line:

QRgb* end = p + image.width();

struck me as particularly clever...

Thanks for testing Paul, that’s good news, I’m going to submit a merge request so we can merge this code in the trunk. And thanks for your thorough investigation and for the original solution!

For information, the version I wrote is largely inspired by the code of QImage::rgbSwapped(), I didn’t really invent anything ;)

That's great news Olivier!
I'm very pleased to have been able to give something back to Ubuntu, not to mention quite proud!

This bug was fixed in the package unity-2d - 3.8.6-0ubuntu1

---------------
unity-2d (3.8.6-0ubuntu1) oneiric; urgency=low

  * New upstream release:
    - [launcher] Support static shortcuts in the quicklists. (LP: #669923)
    - [launcher] Make sure the whole area of the item tile handle mouse events
      (LP: #743402)
    - [dash, launcher, spread] Added a new command line switch (-opengl) that
      triggers the use of a QGLWidget for the QML viewport. (LP: #773397)
    - [launcher] Fix the last shortcut text to display "0", not "10", to be
      consistent with a typical keyboard layout. (LP: #743420)
    - [launcher] Keyboard shortcuts for place entries (LP: #732637)
    - <Meta>+T to open the trash. (LP: #771886)
    - [launcher] Use appropriate icon for devices depending on their type
      (instead of always showing a USB icon). (LP: #703309)
    - [launcher] Grab the Meta+n (for 0 ≤ n ≤ 9) hotkeys at startup, and release
      them only when exiting. (LP: #758650)
    - Fix the byte order of images on Big Endian architectures (like PowerPC).
      Original algorithm by Paul J. Wells, reworked for efficiency.
      (LP: #758782)
    - [launcher] Ensure all delegates are cached in order to improve smoothness
      of scrolling on very low end platforms. (LP: #780566)
    - [dash] Shortcuts expanded state are now persistent. (LP: #774437)
    - [launcher] Match trash nautilus window with the trash item in the
      launcher. (LP: #692444)
    - [launcher] For the workspaces and window overviews, trigger Compiz's Expo
      and Scale plugins if Compiz is running. If Compiz isn't running, said
      plugins aren't enabled or the D-Bus plugin isn't enabled, fall back to
      using unity-2d-spread. (LP: #760674, #715244)
  * debian/control:
    - remove deprecated transitional packages (only from maverick ppa or early
      natty alpha)
    - don't dep on gnome-session-bin
    - remove the -dev package, uneeded for a private library internal to this
      source
    - remove bzr build-dep, we will refresh the pot with dh_translations
  * debian/rules:
    - build with dh7 now
    - buid with --fail-missing and purging installed but not shipped private
      headers
  * debian/unity-2d-launcher.preinst:
    - removed, deprecated as it was for the ppa migration
  * debian/libunity-2d-private0.post*
    - removed: ldconfig is generated directly by dh_makeshlibs as we don't pass
      -n anymore
  * don't install unity-2d session as it's in gnome-session now
  * debian/unity-2d.install
    debian/20_ubuntu-2d-gconf-default
    debian/20_ubuntu-2d-gconf-mandatory
    debian/gconf/ubuntu-2d.default.path
    debian/gconf/ubuntu-2d.mandatory.path:
    - remove uneeded defaults and mandatory
    - remove some deprecated keys
    - the session is now called ubuntu-2d and not unity-2d anymore
  * debian/unity-2d.preinst, debian/unity-2d.postinst
    debian/unity-2d.postrm:
    - take into account the natty -> oneiric migration with session name change
 -- Didier Roche <email address hidden> Wed, 25 May 2011 12:19:25 +0200