Hi Armin, I am not FreeRDP upstream maintainer, but I glad to see you are working on color conversions overhaul. Current state with color conversions seems pretty nonuniform and complicated. I am preparing fixes for big endian architectures, because there are corrupted colors and several other problems. I used following pull request for a discussion: https://github.com/FreeRDP/FreeRDP/pull/3284
Most of the code works on a big endian, because Stream_Read_/Stream_Write_ macros are used to read/write data from/to server. There is a problem if Stream_Pointer is used to read raw data, because the data are always stored as little endian. It is problem especially with wide strings and colors. It seems to me that usage of something like Stream_Read_Bitmap/Stream_Read_String would be suboptimal, because it requires additional data processing, but maybe this is the only right way to do it. Some of the color conversions access the data byte per byte, some of them access them as UINT16, or UINT32 currently. This is obviously a problem for a big endian. It seems that your patches fixes this problem thanks to ReadColor/WriteColor functions, so it is great! Unfortunately there is another problem. X11 client requires "inverted" colors on a big endian (not sure about another clients). I realized that "inverted" color for ARGB32 should be BGRA (not ABGR as it is used currently), however this doesn't work for another bit depths. "Inverted" color for RGB16 is not BGR16, but RGB16 with swapped bytes, i.e. loaded as little endian and stored as big endian. So, I am not sure whether BGR color formats are really needed at all. Don't you know whether some of the clients really need BGR? It seems to me that BGR color formats were intended to handle colors on a big endian, but it was probably misunderstanding. So, maybe the "inverted" color formats (i.e. BGR, ABGR, XBGR) may be removed and something like FREERDP_PIXEL_FORMAT_INVERT may be introduced instead. What do you think about it? Btw it is not possible to build FreeRDP with unit tests (-DBUILD_TESTING=ON), because the tests are broken by your patches. I wanted to test your patches on a big endian also, but xfreerdp segfaulted. I need to take a look why... 2016-04-25 9:26 GMT+02:00 Armin Novak <armin.no...@thincast.com>: > Hi there, > > I'm working on a overhaul of the software GDI color conversion. > My current work is here: > https://github.com/akallabeth/freerdp/tree/color_conversion_fix_v3 > > This pull requires refactoring of most of libfreerdp and there are still > a few issues left (listing later on) > but it is aimed at fixing the following issues: > > * Decouple local framebuffer format from remote one > This will allow having a local 32bit framebuffer for a 8/15/16/24 bit > RDP session > > * Allow arbitrary color ordering of local framebuffer and fix all > RFX/H264/GDI functions to > respect that color order > > * Allow externally supplied buffers to be used in GDI layer > > * Remove some performance bottlenecks (do less copying of input data), > re-factor the touched API to fix some flaws. > > It is working quite well so far for RFX/GFX/H264 but there remain some > problems with GDI mode. > > I'd like some people have a look at it before creating a pull for it, > so if anyone is interested do have a look at the branch. > > best > Armin > > > ------------------------------------------------------------------------------ > Find and fix application performance issues faster with Applications > Manager > Applications Manager provides deep performance insights into multiple > tiers of > your business applications. It resolves application problems quickly and > reduces your MTTR. Get your free trial! > https://ad.doubleclick.net/ddm/clk/302982198;130105516;z > _______________________________________________ > FreeRDP-devel mailing list > FreeRDP-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/freerdp-devel > -- Regards Ondrej ------------------------------------------------------------------------------ Find and fix application performance issues faster with Applications Manager Applications Manager provides deep performance insights into multiple tiers of your business applications. It resolves application problems quickly and reduces your MTTR. Get your free trial! https://ad.doubleclick.net/ddm/clk/302982198;130105516;z _______________________________________________ FreeRDP-devel mailing list FreeRDP-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freerdp-devel
