Bug 145725 - Under Option VBAsupport 1 the RGB() wrongly interchanges the values for Red and Blue.
Summary: Under Option VBAsupport 1 the RGB() wrongly interchanges the values for Red a...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Documentation (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.4.0
Keywords:
Depends on:
Blocks:
 
Reported: 2021-11-16 20:22 UTC by Wolfgang Jäger
Modified: 2021-12-26 12:42 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
The attachment announced in the original report (14.59 KB, application/vnd.oasis.opendocument.spreadsheet)
2021-11-16 20:22 UTC, Wolfgang Jäger
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Wolfgang Jäger 2021-11-16 20:22:24 UTC
Created attachment 176297 [details]
The attachment announced in the original report

RGB(255, 0, 0) is "pure red" under standard conditions.  
It is pure blue as expected for 
RGB(0, 0, 255) if called in a module having enabled Option VBAsupport 1 .
Very Strange! 
Additional examples are contaoined in tzhe attachment. 


The misbehavior was tested and verified down to V4.1.5.3 (from PortableApps package) and is still present in V7.2.1.2.  

See Attachment!
Comment 1 Julien Nabet 2021-11-17 20:08:43 UTC
On pc Debian x86-64 with master sources updated today, I could reproduce this.

The involved method is SbRtl_RGB here:
https://opengrok.libreoffice.org/xref/core/basic/source/runtime/methods.cxx?r=4f5b3e4b&mo=118773&fi=4060#4060
and the VBAOption matters here:
   4075     if( bCompatibility )
   4076     {
   4077         nRGB   = (nBlue << 16) | (nGreen << 8) | nRed;
   4078     }
   4079     else
   4080     {
   4081         nRGB   = (nRed << 16) | (nGreen << 8) | nBlue;
   4082     }

It's like this since:
https://cgit.freedesktop.org/libreoffice/core/commit/?id=9f19bd72abb62c018800976a1232adca64eb1a3c
author	Rüdiger Timm <rt@openoffice.org>	2006-05-05 07:38:28 +0000
committer	Rüdiger Timm <rt@openoffice.org>	2006-05-05 07:38:28 +0000
commit 9f19bd72abb62c018800976a1232adca64eb1a3c (patch)
tree d908f551eb609ba6f5a7a6f06f70b1f3896fee65
parent f986786ef4002cb89ce75884ab2ec996001993cd (diff)
INTEGRATION: CWS ab25 (1.64.24); FILE MERGED
2006/04/18 11:23:10 ab 1.64.24.1: #129905# Changed RGB behaviour for compatibility mode

I must recognize I don't know what's the rationale for this.

Eike/Mike: any thoughts here?
Comment 2 Mike Kaganski 2021-11-17 20:36:05 UTC
https://www.oreilly.com/library/view/vb-vba/1565923588/1565923588_ch07-1860-fm2xml.html

I suppose it is not a bug. The link above explains that the order of the color components in the Long, returned from VBA's RGB, is the opposite of the expected. So it's not the color is blue, it's an MS color incompatible with LO color.
Comment 3 Wolfgang Jäger 2021-11-17 21:08:28 UTC
(In reply to Mike Kaganski from comment #2)
> https://www.oreilly.com/library/view/vb-vba/1565923588/1565923588_ch07-1860-
> fm2xml.html
> 
> I suppose it is not a bug. The link above explains that the order of the
> color components in the Long, returned from VBA's RGB, is the opposite of
> the expected. So it's not the color is blue, it's an MS color incompatible
> with LO color.
I havent access to any MS sogftware with VBA, but...
Even MS would (hopefully) not use the function name RGB() if expeting parameters in the order b, g, r. 
IF MS use a different color model by default in VBA, they need to name and specify their functions respectively. 
But: There is a RGB() function in VBA, 
and it expects the arguments in order r, g, b.

See also https://docs.microsoft.com/en-us/office/vba/language/reference/user-interface-help/rgb-function
Comment 4 Julien Nabet 2021-11-17 21:14:46 UTC
(In reply to Wolfgang Jäger from comment #3)
> ...
> I havent access to any MS sogftware with VBA, but...
> Even MS would (hopefully) not use the function name RGB() if expeting
> parameters in the order b, g, r. 
> IF MS use a different color model by default in VBA, they need to name and
> specify their functions respectively. 
> But: There is a RGB() function in VBA, 
> and it expects the arguments in order r, g, b.
> 
> See also
> https://docs.microsoft.com/en-us/office/vba/language/reference/user-
> interface-help/rgb-function

The pb isn't the order of arguments when calling the function, both LO and MS use R, G and B args in this order but it's a matter of how to store the values in one single Long value.

It's precisely indicated in Mike's link:
"
In other words, the individual color components are stored in the opposite order one would expect. VB stores the red color component in the low-order byte of the long integer's low-order word, the green color in the high-order byte of the low-order word, and the blue color in the low-order byte of the high-order word
"
Comment 5 Julien Nabet 2021-11-17 21:21:09 UTC
I submitted a patch on gerrit here:
https://gerrit.libreoffice.org/c/core/+/125430
to document why it's not a bug.
Comment 6 Wolfgang Jäger 2021-11-17 22:06:04 UTC
(In reply to Julien Nabet from comment #4)
> (In reply to Wolfgang Jäger from comment #3)
> > ...
...
> > But: There is a RGB() function in VBA, 
> > and it expects the arguments in order r, g, b.
> > 
> > See also
> > https://docs.microsoft.com/en-us/office/vba/language/reference/user-
> > interface-help/rgb-function
> 
> The pb isn't the order of arguments when calling the function, both LO and
> MS use R, G and B args in this order but it's a matter of how to store the
> values in one single Long value.
> 
> It's precisely indicated in Mike's link:
> "
> In other words, the individual color components are stored in the opposite
> order one would expect. VB stores the red color component in the low-order
> byte of the long integer's low-order word, the green color in the high-order
> byte of the low-order word, and the blue color in the low-order byte of the
> high-order word
> "
Sorry. 

I simply couldn't imagine such an absurd lack in standards. I should have known better. 

In addition I feel lost considering the consequences for users Of VBA in LibO generally. And what would be the effect if LiBO running code under VBAsupport 1 Would revert the byte order for the RGB result to its own habit. Where are RGB results ever exported to the file except when assigned to a colored object? The visible color is interpreted the LibO way, of course, and written after encoding it the LibO way.
Comment 7 Mike Kaganski 2021-11-18 06:25:09 UTC
(In reply to Wolfgang Jäger from comment #6)
> I feel lost considering the consequences for users Of VBA in
> LibO generally. And what would be the effect if LiBO running code under
> VBAsupport 1 Would revert the byte order for the RGB result to its own
> habit. Where are RGB results ever exported to the file except when assigned
> to a colored object? The visible color is interpreted the LibO way, of
> course, and written after encoding it the LibO way.

This is a very good concern.

MS RGB implementation follows MS COLORREF datatype [1]. So whenever one wants to use the color value in a WinAPI call (which is rather common in VBA), or when one works with file IO, it may be crucial to use specific byte order. So in compatibility mode, there's an interoperability with Windows API, but lack of interoperability with own API working with colors.

OTOH, there's unclear aspect of cross-platform operation. WinAPI (and VBA) are likely defined in terms of their (only) endianness? Which is little-endian (even NT on Alpha was little-endian). Then on big-endian platforms, the byte order should be reversed.

Anyway, I'd simply document it carefully in help (the documentation in code is already done by Julien), and right there in the help would suggest a recommended code that would not depend on the compatibility mode.

[1] https://docs.microsoft.com/en-us/windows/win32/gdi/colorref
Comment 8 Mike Kaganski 2021-11-18 08:16:07 UTC
By the way, the opposite direction functions (Red(Long), Green(Long), Blue(Long)) do not consider compatibility mode (naturally, because there's no counterparts in VBA) - and that creates additional confusion.

Also there's now a ScriptForge wrapper to this: svc.RGB [1]. But that would likely not be affected, since wrapped Basic code would not be run in a compatibility mode...

Clear documentation is badly needed in [2]. Also [3], [4] and [5] need a notice that they may return unexpected results on values created using RGB in compat mode.

I'd suggest this code to be suggested as a robust user replacement:

Function MyRGB(r As Integer, g As Integer, b As Integer) As Long
  MyRGB = (r And 255) * 65536 + (g And 255) * 256 + (b And 255)
End Function

[1] https://help.libreoffice.org/latest/en-US/text/sbasic/shared/03/sf_basic.html?DbPAR=BASIC#bm_id831618907521168
[2] https://help.libreoffice.org/latest/en-US/text/sbasic/shared/03010305.html?DbPAR=BASIC
[3] https://help.libreoffice.org/latest/en-US/text/sbasic/shared/03010303.html?DbPAR=BASIC
[4] https://help.libreoffice.org/latest/en-US/text/sbasic/shared/03010302.html?DbPAR=BASIC
[5] https://help.libreoffice.org/latest/en-US/text/sbasic/shared/03010301.html?DbPAR=BASIC
Comment 9 Mike Kaganski 2021-11-18 08:50:30 UTC
Also, since [1] lists affected functions, RGB may be added there, too.

[1] https://help.libreoffice.org/latest/en-US/text/sbasic/shared/compatibilitymode.html?DbPAR=BASIC
Comment 10 Commit Notification 2021-11-18 09:01:55 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/ccfeef1a48f5add83e443afd6664f40df59381b4

tdf#145725: document about VBASupport+RGB (no bug here)

It will be available in 7.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2021-12-20 12:11:14 UTC
Olivier Hallot committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/help/commit/fb0af5ca08c6c9cff2223e641dbd73f9cd2c47c0

tdf#145725 VBA mode for RGB functions and warnings