12
\$\begingroup\$

This is how I convert e.g. RGB(255, 0, 0) (where red is stored in the least significant byte of a Long) to a 6-digit hex string FF0000:

Function RgbToHex(l As Long) As String
Dim r As Long
Dim g As Long
Dim b As Long
    r = l And ColorConstants.vbRed
    g = (l And ColorConstants.vbGreen) / (2 ^ 8)
    b = (l And ColorConstants.vbBlue) / (2 ^ 16)
    RgbToHex = Right$("0" & Hex(r), 2) _
             & Right$("0" & Hex(g), 2) _
             & Right$("0" & Hex(b), 2)
End Function

I'm a bit worried about the string manipulations, but as VB6 lacks printf-like formatting, this is the best I could come up with. Can it be improved?

\$\endgroup\$
  • \$\begingroup\$ What worries you about the string manipulations? Performance? \$\endgroup\$ – Comintern Dec 14 '16 at 17:39
  • \$\begingroup\$ If it's not about performance, you might want to take a look at this StringFormat function that I wrote a few years ago in VB6, to format strings exactly like one would in C#/.NET. \$\endgroup\$ – Mathieu Guindon Dec 14 '16 at 18:16
14
+500
\$\begingroup\$

The code you have works, and is very nicely-formatted.

There are only two things I would change, stylistically speaking:

  1. Explicitly pass input parameters by value. This should have been the default, but it wasn't in classic COM-based VB, so you need to write it out. ByRef is actually a perverse choice for the default, since it is almost never what you want. It is certainly not what you want for primitive types like Long. (But it probably is what you want for strings, unless passing them out-of-process, e.g. in a COM server.)

  2. As others have pointed out, l is a rather poor choice for a parameter name. Roland says one problem with it is that it can be difficult to distinguish from the digit 1 or a capital I. I'd say that's more a reflection on your choice of a font for your editor, but this is still a valid concern. My major issue with it is that it looks like a lame attempt at Systems Hungarian notation. I know this used to be all the rage in VB programming, but that was mostly cargo-cult. Unless you're using variants (and you probably shouldn't be) the language is sufficiently strongly-typed that embedding type information in the name of the variable serves little point. The only type prefixes I allow myself are for names of controls (mostly because it makes finding them in IntelliSense significantly easier; I always remember the type of control much more readily than I remember its name) and "member" prefixes (e.g., m_). Aside from that, the only prefixes you should be using are meaningful semantic ones, in the Apps Hungarian vein.

Therefore, I would rewrite your function's prototype to look like the following:

Function RgbToHex(ByVal color As Long) As String

Otherwise, the primary issue with your code is that it is slow!

Why? Well, there are a couple of reasons:

  1. String operations are extremely slow in VB since they all require multiple BSTR allocations and deallocations, not to mention the fact that the calls to the string-manipulation functions themselves cannot be inlined.

    Now, I notice that you did correctly force the use of the string-based Right function by using the $ type suffix, which is a standard trick to ensure as optimal code as possible. This is good. You didn't do the same thing for Hex. Personally, I have no idea whether it is actually necessary or not—is there a variant-based version of Hex? Maybe, I don't know. And I don't want to have to worry about it when I write or review code, so I always use the $ type suffix when I am working with strings. I suggest that you do the same. (Mat's Mug says Hex does have a variant version, so looks like this is sage advice after all!)

    But the major problem here is that you're calling six different string-manipulation functions, plus concatenating those three intermediate strings. Slooooow! To optimize the code, it will be critical that we find a way to keep these calls to a minimum.

  2. You are using floating-point operations in your byte-extraction code.

    In your defense, constant expressions like 2 ^ 8 should be computed by the compiler at compile-time, so that your code never actually has to do an exponentiation operation. I would have made exactly that same assumption. Unfortunately, the VB 6 compiler (even with all optimizations enabled) is not that smart. These two exponentiation operations are done at run-time, with a call to the vbaPower function exported by the run-time library. Personally, I'd say this is a bug in the compiler, but it's not going to get fixed now, and you have no choice but to work around it.

    The other bad news is, once you've got a floating-point value, the entire arithmetic operation has to be done in floating-point mode. So for each of your bit-extraction operations, the compiler is going to emit code that converts an integer value into a floating-point value, loads floating-point constants, performs an exponentiation operation, fixes up the result, and then does a division.

    While it is true that floating-point operations are no longer as slow on modern machines as they used to be, generally obsoleting the classic optimization advice to avoid floating-point operations whenever possible, VB 6 is not sufficiently modern for that advice to hold. :-) There is no way to get the VB 6 compiler to emit SSE instructions, so you're stuck with the legacy x87 FPU, and that means that you're stuck with a bottleneck intrinsic in its design, arising from the inability to move values directly from the floating-point stack into integer registers and vice versa. The compiler emits code to handle all of this for you seamlessly, but it's still slow. So, at least in the context of VB 6, it's still valid optimization wisdom not to inter-mix floating-point and integer operations.

    In this case, though, you don't need floating-point operations at all! You could have done everything in integer mode, just by explicitly using an integer division:

    r = (l And ColorConstants.vbRed)
    g = (l And ColorConstants.vbGreen) \ (2 ^ 8)
    b = (l And ColorConstants.vbBlue)  \ (2 ^ 16)
    

    Then, the constants even get folded correctly, with the exponentiation being done at compile-time.

As originally written, your function compiles to hundreds of lines of machine code. I made an optimized build and then disassembled it, revealing a total of over 1,000 bytes of machine code just for that one function. It contains tons of embedded function calls (slow), string allocations/deallocations (slow), and extremely sub-optimal code to store, compare, and manipulate floating-point values (slow).

Let's have a go at optimizing it.

Comintern made one attempt already, and it is interesting to study. Immediately one problem jumps out—it does not produce correct results. Specifically, the order of the red and blue component values are reversed, such that instead of AABBCC, his code gives CCBBAA. On little-endian architectures, like the x86, integer values are stored in memory with the least-significant byte at the smallest address. This means that "RGB" colors are actually represented in memory using an BBGGRR format, whereas you are wanting the standard, web-style RRGGBB format for your hexadecimal output. This is a minor bug, though, and should be fixable.

My real issue with his code is that it makes a series of function calls, specifically to the RtlMoveMemory API function, which he has aliased to CopyMemory. I've seen this technique before for optimizing code in VB 6, and while it has power, it's not a panacea. Why? Function calls are expensive, too! Memory copying is a relatively cheap operation, O(n) at the worst, and has a small constant multiplier assuming an optimal implementation (which RtlMoveMemory does provide), but the context switching required for a function call introduces a lot of overhead that it would be nice to avoid. This is especially important in a very small, do-one-simple-thing function like you have here. VB 6's compiler does a very poor job of inlining code, so the execution cost of this function would be dwarfed by the overhead cost of making these system calls.

To be fair, Comintern's code is still going to be significantly faster than your original attempt, but we can do even better.

How? By using bit-manipulation tricks to do the byte-swapping. Basically, you do what you would have done in C (or at least, what I would have done in C, maybe you're a real VB programmer and don't think in the language of blasphemy). This is basically what Roland Illig did in his code. You start with the input value, in AABBGGRR format. Then, you isolate each of the individual color components (except the alpha channel, which we don't care about):

  • The red component is trivial: you just mask off all but the bottom-most byte. Bitwise masking is done with a bitwise And operation.
  • For the green component, you first divide by &H100 (which is the same as a right-shift by 8, except that you can't do a right-shift in VB). That throws away the bottom-most byte, shifting everything down by one byte. Then you can simply mask off all but the bottom-most byte, which is the one that used to be the middle byte.
  • The same thing is true for the red component, except that we divide by a larger constant, &H10000, to achieve what is effectively a right-shift by 16.

My hope/expectation was that the VB 6 compiler would actually optimize these division operations to right-shift operations, since that will be faster—and indeed, that is exactly what it did! Notice that it is critical that we use the integer-multiply operator, \, here.

The next step is to take those isolated byte values, and pack them together in the proper order. Again, Roland's code does the same thing I would have done: uses a multiplication in lieu of a left-shift operator. Unfortunately, VB 6's compiler can't/won't optimize this multiplication into a left-shift. Fortunately, multiplications are not slow on modern processors, so this is not a big loss.

Finally, we want to make a single call each string-formatting function to pretty-print the final output. Roland's idea was to multiply the integer value by &H1000000 to ensure that it is properly zero-padded—very clever! I did it a bit differently: I decided to check and see if padding is actually necessary, and if not, skipping the call to Right altogether!

Public Function RgbToHex(ByVal color As Long) As String
    ' On little-endian architectures, "RGB" color values are actually stored in memory
    ' in an AABBGGRR format. However, web-style hex colors are always formatted in an
    ' AARRGGBB format. Neither Windows nor HTML care about the alpha-channel value
    ' (always the upper, most-significant bit), so we are going to simply ignore it.
    ' Otherwise, we have to do some bit-twiddling to swap the red and blue color channels.

    ' Extract the individual red, green, and blue color channels.
    ' (Although they are only bytes, continue to store these intermediate values
    ' as 32-bit integers for maximum speed.)
    Dim r As Long
    Dim g As Long
    Dim b As Long
    r = (color And &HFF)
    g = ((color \ &H100) And &HFF)
    b = ((color \ &H10000) And &HFF)

    ' Now, arrange these byte values in the correct order (RRGGBB).
    color = ((r * &H10000) + (g * &H100) + b)

    ' And finally, format it as a hexadecimal string, left-padding with zeros as necessary.
    ' Note that if the red component value is large enough, padding does not need to be
    ' done, which allows us to skip an expensive string operation.
    If (r >= &H10) Then
        RgbToHex = Hex$(color)
    Else
        RgbToHex = Right$("00000" & Hex$(color), 6)
    End If
End Function

If this were C, I would guess that Roland's solution would probably be faster, since it does not involve a conditional branch (and conditional branches can be expensive when they are not properly predicted). However, since this is VB, I suspect that my code will be slightly faster, mainly because string operations are so incredibly expensive that it is worth a mispredicted branch penalty to elide one if at all possible. And, if I may compliment myself, I'd say that my solution is slightly more readable from a code-review perspective. (But either solution would require explicit comments describing the rationale.)

Notice, also, that in my implementation, I have a comment about choosing to keep the intermediate values in a 32-bit Long, even though they are just bytes and could be represented using the Byte type. The reason for this is that it is inherently faster to work with 32-bit values on 32-bit architectures, since they fit directly into the processor's native 32-bit wide registers. (This is why you should always use Long in VB 6, rather than the 16-bit Short type. You pay an additional penalty when working with 16-bit values!) If we'd tried to store them as Bytes, they'd take up less memory, but the realistic limitations of VB 6's optimizer mean that there will be some unnecessary conversion going on. If VB supported unsigned types, using them would open up even more potential for the optimizer to do its magic, especially when doing bit-twiddling operations; alas, it doesn't, so we're stuck with signed types.

Can we make it even faster?

One quick and easy way to speed up the code is to change the compiler flags. Too few people do this in VB 6, since it is so well hidden. When building your EXE (File → Make *.exe), click the Options button to bring up a dialog. Switch to the "Compile" tab. "Compile to Native Code" is selected by default, and so is "Optimize for Fast Code", which are both good for speed. However, you should also turn on "Favor Pentium Pro" and "Create Symbolic Debug Info".

Creating debug info is just a good idea, in case you ever have to debug your application, and it is impossible to generate it later. This is arguably less useful for a strictly VB programmer who wouldn't know how to use another debugger with a gun to his head, but it is a skill that is truly worth learning, and if you ever have to call in an expert, a PDB file is invaluable. It doesn't hurt anything to generate it and ignore it.

Why favor the Pentium Pro? Granted, this sounds like a hyper-specific option to favor a certain obsolete family of microprocessor, and indeed it was at the time that VB 5 was released, but it is actually the best general choice because all modern processors bear far more similarity to the Pentium Pro than they do to other generations of processors (with the exception of the Pentium 4, but VB 6 can't optimize for that anyway, and optimizing for PPro is still not a pessimization). The Pentium Pro was a unique processor in a number of ways, but two things were most important from an optimization sense. First, it supported out-of-order execution, where individual instructions could be dynamically re-ordered by the processor to overcome instruction-level data dependencies. Thus, it is better if the compiler generates shorter sequences of instructions (RISC-style), because these can be better reordered. Second, the Pentium Pro has a stall for certain partial-register accesses. The details are complicated and beyond the scope of this answer, but the important fact is that while the earlier Pentium didn't have these, all modern processors do, so having the optimizer generate code accordingly is a good idea.

Other important checkboxes are buried behind the "Advanced Optimizations" button. The UI tries to make these sound scary, but if you understand what they do, and your code has been truly and correctly debugged, some of them are worth enabling. Most obviously, you can turn on the bottom option, "Remove Safe Pentium FDIV Checks". The early Pentium processors with the infamous FDIV bug are long dead now, so this is just pointless baggage to carry around. The other options actually can be slightly unsafe, but it is worth considering turning on the "Remove Integer Overflow Checks" option, and possibly the "Remove Floating Point Error Checks" and "Allow Unrounded Floating Point Operations" options. These three options have a massive impact on the object code that is generated by the compiler, and enabling them can cause a correspondingly massive speed-up in the execution time of your code.

As an example, when I threw the "Remove Integer Overflow Checks" switch and recompiled my code, I saw that it had used shift instructions, instead of multiplication instructions, like I earlier wished that it had! Better yet, removing these overflow checks means that the compiler no longer emits conditional branches after each integer (or floating-point) operation, and even though these branches will be properly predicted to fall through most of the time, leaving them out entirely is still faster. This is a cheap way to make your code run a lot faster, without having to rewrite any of it.


For the hardcore among you, here is my annotated disassembly for the final code generated by the VB 6 compiler with optimizations enabled. You should be able to see some of what I'm talking about in terms of the rather-optimal bit-twiddling instructions, as well as the less-than-optimal string-manipulation code. But being completely overwhelmed is also normal.

\$\endgroup\$
  • \$\begingroup\$ Good information on the VB6 compiler options. It makes me curious what the VBA compiler does (although I'm guessing it's no optimizations at all). Thinking in C, though I'd probably do a union or an unsafe cast to *char though ;-). \$\endgroup\$ – Comintern Dec 16 '16 at 13:56
  • \$\begingroup\$ @com VBA, as far as I'm aware, compiles exclusively to P-code. It doesn't come with a native compiler. That's the big difference between it and VB 6. As such, completely different performance concerns would apply; you want short code that can be easily analyzed by the interpreter, not necessarily complex code that would result in fast object code once compiled. As far as what's good in C, bit-shifting is still the best by a landslide. The compiler might emit bit-shifting code for a union, but if it does memory copies, you're back to slow zone. \$\endgroup\$ – Cody Gray Dec 16 '16 at 14:10
  • \$\begingroup\$ It's stored as pcode, but at some point pcode is converted to opcodes (MS hasn't exactly made the process transparent), because it's sent through the kernel debugging APIs with symbols attached. That's just semantics though. I'm unclear on how a union or an unsafe cast to *char would ever involve a memory copy that wasn't a coding error though. \$\endgroup\$ – Comintern Dec 16 '16 at 14:26
  • 1
    \$\begingroup\$ That doesn't mean the runtime won't make a copy of the BSTR and pass that pointer, @felix. The information I see on the web confirms that modifying a string passed ByVal won't propagate that change back to the caller, which means it has to be making a copy. \$\endgroup\$ – Cody Gray Dec 16 '16 at 16:22
  • 4
    \$\begingroup\$ I have two words: Epic, Answer. You will be earning a little bounty soon-ish for it. Also.. on behalf of the Rubberduck team I'd like to invite you to come chat in VBA Rubberducking anytime you like. Cheers! And these epic answers... keep 'em coming! \$\endgroup\$ – Mathieu Guindon Dec 16 '16 at 22:50
12
\$\begingroup\$

I'll just focus on performance, because that's the fun part.

You're correct to be worried about the string manipulations, so you should obviously try to minimize them. The other thing you should be worried about is the division and exponentiation used for bit-twiddling. If all you need to do is swap the byte order of an intrinsic type, you can do it using a CopyMemory API call, a struct, and simple assignments:

'Note that this is a VBA declaration (missed the VB6 tag) - adjust accordingly.
Private Declare PtrSafe Sub CopyMemory Lib "kernel32" Alias "RtlMoveMemory" _
            (destination As Any, source As Any, ByVal length As LongPtr)

Private Type LongBytes
    Byte1 As Byte
    Byte2 As Byte
    Byte3 As Byte
    Byte4 As Byte
End Type

Public Function SwapEndian(inVal As Long) As Long
    Dim bytes As LongBytes, swapped As LongBytes

    CopyMemory bytes, inVal, 4

    swapped.Byte1 = bytes.Byte4
    swapped.Byte2 = bytes.Byte3
    swapped.Byte3 = bytes.Byte2
    swapped.Byte4 = bytes.Byte1

    Dim retVal As Long
    CopyMemory retVal, swapped, 4
    SwapEndian = retVal
End Function

Even with the overhead for the API call, this is blazingly fast. It also completely eliminates the need to call Hex repeatedly, completely eliminates the need to pad every byte to 2 characters individually, and involves exactly zero math operations.

Once you have the Long in the byte order you want, all you need to do is call Hex$ once (note that it also has a String returning variation - kudos on using the one for Right$):

Public Function RgbToHex2(inVal As Long) As String
    Dim bytes As LongBytes, swapped As LongBytes

    CopyMemory bytes, inVal, 4
    swapped.Byte1 = bytes.Byte3
    swapped.Byte2 = bytes.Byte2
    swapped.Byte3 = bytes.Byte1
    swapped.Byte4 = bytes.Byte4

    Dim retVal As Long
    CopyMemory retVal, swapped, 4

    RgbToHex2 = Right$("0" & Hex$(retVal), 6)    '<--1 call each to Right$ and Hex$
End Function

I'd ballpark that it runs about 5 times faster.

\$\endgroup\$
  • 1
    \$\begingroup\$ The slow parts are the calls to Hex and Right. It seems that you could further speed up the code by removing the overhead of the calls to CopyMemory and replacing them with simple bit manipulation. With optimization enabled, these operations should be transformed into very fast code. Math operations are definitely going to be faster than an API call. \$\endgroup\$ – Cody Gray Dec 14 '16 at 19:13
  • \$\begingroup\$ @CodyGray - Probably true I'm testing in VBA (so I can't benchmark the optimizations). I'd be curious to see how bit-shifting with division compares to CopyMemory. \$\endgroup\$ – Comintern Dec 14 '16 at 19:23
  • 1
    \$\begingroup\$ Take this one step further and create a 3 byte RGB Type and you've got a great API to work with that doesn't represent RGB as a long. Just food for thought. \$\endgroup\$ – RubberDuck Dec 15 '16 at 1:49
  • 3
    \$\begingroup\$ I started to play around with this, and upon closer inspection, your code (specifically, RgbToHex2) is not actually correct. You cannot simply reverse the byte order of the long value. In memory on Windows, RGB colors are represented as AABBGGRR, whereas in the hex format, they are AARRGGBB. So you should only be swapping the order of the red and blue components. Felix's original code is correct, but yours produces a different answer. \$\endgroup\$ – Cody Gray Dec 16 '16 at 10:55
  • 1
    \$\begingroup\$ @CodyGray - Corrected. (I wasn't really concentrating on the byte order when I banged this out). \$\endgroup\$ – Comintern Dec 16 '16 at 13:40
10
\$\begingroup\$

Obviously @Comintern knocks out the bit-swapping part, so this answer is more about, well, every other aspect of the code.


Signature

Kudos for proper PascalCase!

If the function is meant to be Public, it's probably best to specify it as such. I like that it has an explicit return type, but the parameter is implicitly passed by reference, which is potentially confusing since the function is meant to be "pure" and thus, it's really used as a ByVal parameter and should be specified as such, too. l is a terrible name for it though.

I'd go with this:

Public Function RgbToHex(ByVal rgbValue As Long) As String

That way none of it is ambiguous or implicit in any way.


Variables and Constant Expressions

It's only 3 of them so it's probably not big of a deal, but I've seen way too many VBA procedures with a page-length wall-of-declarations at the top to skip this: variables should be declared as close as possible to their usage.

This reads more fluently IMO:

Dim rValue As Long
rValue = rgbValue And ColorConstants.vbRed

Dim gValue As Long
gValue = (rgbValue And ColorConstants.vbGreen) / (2 ^ 8)

Dim bValue As Long
bValue = (rgbValue And ColorConstants.vbBlue) / (2 ^ 16)

I like how 2 ^ 8 and 2 ^ 16 help readability, but not how they need to be computed every time the function is called. Naming them and introducing constants will maintain the readability while avoiding the recalculation:

Const gOffSet As Long = 2 ^ 8
Const bOffSet As Long = 2 ^ 16

And then:

Dim gValue As Long
gValue = (rgbValue And ColorConstants.vbGreen) / gOffSet

Dim bValue As Long
bValue = (rgbValue And ColorConstants.vbBlue) / bOffSet

Strings

Kudos for using the string-returning Right$ function: that avoids an implicit conversion from Variant that Right would incur.

However Hex also has a string-returning little brother; you should prefer Hex$ over it for the same reason you're using Right$ over Right.

I like how you're using line continuations to line up the function calls, it's very elegant. Sad that it's moot with proper bit swapping :)

\$\endgroup\$
6
\$\begingroup\$
Function RgbToHex(color As Long) As String
    Dim r As Long, g As Long, b As Long
    r = color And &HFF
    g = (color / &H100) And &HFF
    b = (color / &H10000) And &HFF
    RgbToHex = Right$(Hex$(&H1000000 + &H10000 * r + &H100 * g + b), 6)
End Function

Assuming the VB6 compiler really doesn't inline constant expressions, this is probably one of the fastest ways to implement RgbToHex.

Instead of using a mixture of ColorConstants and magic numbers, I have restricted my code to magic numbers only. And these numbers are not really magic, since they are all powers of 2 or very close to them.

Instead of converting every color channel separately to a hex number, it is more efficient to first combine the color channels and then convert them once.

I renamed l to color since a lowercase ell is hard to distinguish from the digit 1 or a capital i.

The number &H1000000 makes sure that the output is always 6 digits long, by producing a 7-digit hex number first and then only keeping the last 6 hex digits of that number. If this number were not there, black would be output as 0 instead of 000000.

\$\endgroup\$
5
\$\begingroup\$

@Comintern got you some performance, and @MatsMug reviewed your code, but nobody had properly addressed your concern over the strings.

Every Right$ call requires a concatenation of "0" and the Hex$ value. That concatenation can be sped up by using a bit-trick:

Right$(Hex$(256 Or r), 2)

Adding 256 is the same effect as Right$("10" & Hex(r), 2) but because it's arithmetic, it will be faster than concatenating stings.

\$\endgroup\$
  • \$\begingroup\$ "Huh?! Oh, right!" <~ exact thought process. not sure that's a good thing though. \$\endgroup\$ – Mathieu Guindon Dec 14 '16 at 19:27
  • \$\begingroup\$ @Mat'sMug That fix should make it clearer? \$\endgroup\$ – ThunderFrame Dec 14 '16 at 19:53
  • \$\begingroup\$ Still feels like it's written for performance over readability. I wouldn't write it that way I think. Not without a comment saying why it matters =) \$\endgroup\$ – Mathieu Guindon Dec 14 '16 at 20:10
3
\$\begingroup\$

Time Measurements!

I tested all of the other answers' code and want to publish the results. This might help compare the different implementations and maybe tell us something about how to optimize VB6 code.

To measure this, I let each function calculate all colors from 0 to &HFFFFFF, once in an EXE compiled just for speed and once in one compiled with all optimizations. My system is an AMD Phenom II X4 960. In measurements on a modern i7, Cody's version was closer to Unknown user's. The full source is here.

Results

Version           Optimized:  only for speed       for speed, PPro, all advanced
----------------------------------------------------------------------------------
Original                        40.101 s                   40.556 s
Mat´s Mug                       32.531 s                   32.154 s
Thunderframe                    27.007 s                   26.893 s
Comintern                        7.871 s                    7.738 s
Roland Illig                     5.905 s                    5.639 s
Cody Gray/Roland Illig           5.342 s                    5.300 s
Cody Gray                        3.389 s                    3.261 s
Unknown user                     2.815 s                    2.700 s

The Cody Gray/Roland Illig data set refers to Cody's solution but without the conditional. The difference to Roland's version is the use of integer division. The impact of just using the conditional is really enormous (2 s!); of course enumerating the colors in this order plays extremely well with branch prediction.

The Unknown user data set refers to the answer posted by a new user whose name I forgot. The answer got deleted, but I saved it and I feel it is a valuable addition to this question. It used a trick to calculate the actual hex chars from the RGB byte values, and I replicate it below.

Strangely, my original code, while of course being the slowest, was even slower with optimizations turned on. This was reproducable in every run, but overall the optimization effects are smaller than I expected.

Unknown user's original answer

Here's a somewhat cryptic version of RgbToHex I just wrote to demonstrate that we can actually do it internally with minimal external function calls (should be just the fixed string to BSTR at the end requiring an external call, edit: unless the LSet memory copies are done externally...), no conditional branching, and no slow string manipulation.

It should offer decent performance for those reasons, but I haven't benchmarked it against the other excellent answers you've already got.

 Type Struct_Color
    Color As Long
End Type

 Type Struct_RGBABytes
    R As Byte
    G As Byte
    B As Byte
    A As Byte
End Type

 Type Struct_RGBALongs
    R As Long
    G As Long
    B As Long
    A As Long
End Type

 Type Struct_FixedString6
    FixedLenStr As String * 6
End Type

Public Function RgbToHex(ByVal ColorRGB As Long) As String

    Dim InputColor                      As Struct_Color
    Dim InputRGBAccessor                As Struct_RGBABytes
    Dim OutputRGBAccessor               As Struct_RGBALongs
    Dim IntermediateOutput              As Struct_FixedString6

    Const LowerCaseHex                  As Boolean = False
    Const AsciiZero                     As Long = 48                        ' Asc("0")
    Const AsciiTenToAOffset             As Long = 7 + (LowerCaseHex * -32)  ' Asc("A") - (Asc("9") + 1) = 7
    Const HalfByteDivider               As Long = 16                        ' divider to do right shift by 4 bits
    Const LowerToUpperWordMultiplier    As Long = &H10000

    InputColor.Color = ColorRGB                                 ' copy the RGB Long value to a simple struct container
    LSet InputRGBAccessor = InputColor                          ' memory copy of struct to allow access to the individual R/G/B values

    ' convert each R/G/B value to a DWORD/Long containing the equivalent of 2 unicode characters (the high and low bits of each byte, each converted to hex)
    OutputRGBAccessor.R = AsciiZero + (InputRGBAccessor.R \ HalfByteDivider) + (((InputRGBAccessor.R \ HalfByteDivider) \ 10) * AsciiTenToAOffset) + _
                            (AsciiZero + (InputRGBAccessor.R Mod HalfByteDivider) + (((InputRGBAccessor.R Mod HalfByteDivider) \ 10) * AsciiTenToAOffset)) * LowerToUpperWordMultiplier
    OutputRGBAccessor.G = AsciiZero + (InputRGBAccessor.G \ HalfByteDivider) + (((InputRGBAccessor.G \ HalfByteDivider) \ 10) * AsciiTenToAOffset) + _
                            (AsciiZero + (InputRGBAccessor.G Mod HalfByteDivider) + (((InputRGBAccessor.G Mod HalfByteDivider) \ 10) * AsciiTenToAOffset)) * LowerToUpperWordMultiplier
    OutputRGBAccessor.B = AsciiZero + (InputRGBAccessor.B \ HalfByteDivider) + (((InputRGBAccessor.B \ HalfByteDivider) \ 10) * AsciiTenToAOffset) + _
                            (AsciiZero + (InputRGBAccessor.B Mod HalfByteDivider) + (((InputRGBAccessor.B Mod HalfByteDivider) \ 10) * AsciiTenToAOffset)) * LowerToUpperWordMultiplier

    LSet IntermediateOutput = OutputRGBAccessor                 ' copy the unicode equivalent struct over into a fixed-length string
    RgbToHex = IntermediateOutput.FixedLenStr                   ' convert the fixed-length string on the stack into a real BSTR.

End Function
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks for putting some hard numbers to this. The rankings are almost exactly what I would have expected, which makes me feel pretty good about the performance analyzer between my ears. :-) I hadn't seen "unknown user"'s solution (very interesting); it serves as an excellent proof of the inherent inefficiency of VB 6's built-in string manipulation. You could almost certainly do even better by importing some WinAPI functions and doing all of the string manipulation that way, but you get into a very real tradeoff between readability and maintainability, vs. real-world performance benefits. \$\endgroup\$ – Cody Gray Dec 23 '16 at 14:48
  • \$\begingroup\$ ... At the risk of sounding arrogant, I would say that my solution strikes the appropriate balance there. Anyone can understand what the code is doing, it looks like it's doing the right thing, but it isn't necessarily the fastest. Of course, there is no such thing as the "fastest" code—anything can be improved with enough effort! The only thing about the numbers that surprised me was that there wasn't more improvement seen by enabled advanced optimizations. Like you said, though, successful branch prediction is playing a huge role here, so this may not be the best general benchmark. \$\endgroup\$ – Cody Gray Dec 23 '16 at 14:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.