#156 Additional GDI function encapsulations in TDC

[Chris Kohlert]

hi, i added some missing wrappers for functions in the Device Context API domain that were introduced with windows 2000 and win98.

The following functions were added in [r5003]:

• GetDCPenColor
• SetDCPenColor
• GetDCBrushColor
• SetDCBrushColor
• GetGlyphIndices
• GetTextExtentPointI
• GetTextExtentExPointI
• GetCharWidthI
• GetCharABCWidths
• GetCharABCWidthsI
• ColorCorrectPalette

The following were added in [r5024]:

• WindowFromDC
• GetColorAdjustment
• SetColorAdjustment
• ArcTo
• GetArcDirection
• SetArcDirection
• SetPixelV
• GradientFill
• AlphaBlend
• GetCharABCWidthsFloat
• GetCharWidthFloat
• GetFontUnicodeRanges

Note that this revision includes an implementation of [feature-requests:#79] (GradientFill and AlphaBlend), with accompanying convenience functions GradientTriangleFill and GradientRectFill.

[Chris Kohlert]

and i have a question there: how much c++ is wanted in those wrappers?

for example a wrapper for this API:

BOOL GetTextExtentExPointI(
  HDC    hdc,
  LPWORD lpwszString,
  int    cwchString,
  int    nMaxExtent,
  LPINT  lpnFit,
  LPINT  lpnDx,
  LPSIZE lpSize
);

https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-gettextextentexpointi

there are 4 pointers that can possibly go wrong and 4 basic usecases i can imagine:

usecase 1: we give in a list of glyphs and only want the bounding box - here we could replace lpwszString + cwchString with a std::vector<WORD> to minimize the parametercount and improve security:

inline TSize TDC::GetTextExtentExPointI(const std::vector<WORD> &glyphString) const {
  int maxExtent; // gets ignored...
  return GetTextExtentExPointI(glyphString.data(), glyphString.size(), maxExtent, nullptr, nullptr);
}

usecase 2: we give in a list of glyphs and we want the range of glyphs that could fit into some range of logical units. we omit the uninteresting pointer lpnDx
from the declaration:

inline TSize TDC::GetTextExtentExPointI(const std::vector<WORD> &glyphString, int maxExtent, uint &maxncFit) const {
  return GetTextExtentExPointI(glyphString.data(), glyphString.size(), maxExtent, maxncFit, nullptr);
}

case 3: maybe we want it even more easy than that and allow the vector to shrink to the size of only fitting glyphs?

inline TSize TDC::GetTextExtentExPointI(std::vector<WORD> &glyphString, int maxExtent) const {
  uint maxncFit;
  TSize size;
  if (::GetTextExtentExPointI(GetAttributeHDC(), const_cast<LPWORD>(&glyphString.data()), glyphStringSize, maxExtent, maxncFit, nullptr, &size)) {
    glyphString.resize(maxncFit);
    return size;
  }
  // function failed
  glyphString.clear();
  return TSize();
}

usecase 4:
we are really interested not only in the amount of fitting glyphs, but in unit delta positions of the glyphs:

inline TSize TDC::GetTextExtentExPointI(const std::vector<WORD> &glyphString, int maxExtent, uint &maxncFit, std::vector<WORD> &glyphExtents) const {
  glyphExtents.resize(glyphString.size());
  TSize size;
  if (::GetTextExtentExPointI(GetAttributeHDC(), glyphString.data(), glyphString.size(), maxExtent, maxncFit, glyphExtents.data())) {
    glyphExtents.resize(maxncFit);
    return size;
  }
  // function failed
  glyphExtents.clear();
  return TSize();
}

this way we could resize the partitialGlyphs ourselfes to ensure that the buffer cant possibly be to small...

where does the abstraction end? :D

[Vidar Hasfjord]

Hi Chris, great work! Good of you to include documentation as well. I didn't know these functions even existed.

Quick review:

• The TSize-returning overload of GetTextExtentExPointI calls itself, rather than the raw overload, and has no error handling (ignores an error code).
• GetTextExtentPointI (LPWORD, int) has no error handling (ignores an error code).
• Some implementations use GetAttributeHDC and others use GetHDC to refer to the underlying handle. It seems inconsistent and arbitrary. E.g. SetDCPenColor uses GetHDC, while SetDCBrushColor uses GetAttributeHDC.
• Don't pass buffers as references to the first element. Either pass a reference to the whole buffer (which makes the function a template on the buffer size), or pass a pointer to the first element and the buffer size, in the conventional way (in the future those two parameters can be wrapped up in a range view object, with the support for ranges coming in C++20).
  
  

Now, some comments on API design:

how much c++ is wanted in those wrappers?

The 7 series wants lots! :-)

It sounds like all your overloads make sense, since the underlying function has so much multi-functionality. But don't return an empty TSize to signal error. If that should not happen, i.e. it is an exceptional error, throw an exception instead. If the client code does not want exceptions, and want to do explicit error checking (which it probably won't anyway), it can use the raw C-style overload.

Looking at this issue more generally, the OWL tradition is to provide a very thin encapsulation, mainly just wrapping an underlying handle. Then we've added more overloads that makes usage simpler and safer, such as the overloads for standard C++ strings, and sometimes we've added utility types to deal with complex parameters (the Windows API has a propensity for multi-functionality functions with large and complex parameter pack structure types).

While the thin wrapping makes little difference in usage (e.g. dc.EndPath() vs EndPath(dc)), it does allow discoverability with modern code editors, i.e. you press period and the list of member functions to choose from pops up. That's good. Having the member function also match the Windows API name and function parameters more or less 1-to-1 is also good for understanding the implementation and cross-referencing the Windows API documentation. In other words, there is no obscuring encapsulation. If you understand the Windows API, you understand OWL.

However, there are problems with exposing the Windows API. First and foremost, it is C. It is not C++. So it has all the C pitfalls with pointers, buffers and (overlooked) error return codes.

For example, take that simple EndPath. Should TDC::EndPath return the Windows API error code, or should it throw an exception on error? It currently returns the error code (bool), but I bet it is rarely checked in most client code. That's bad.

Get-functions in the Windows API usually return the result in an out-parameter via pointer, while the function itself returns an error code. For example, TDC::GetClipBox faithfully replicates this API, but changes the rectangle parameter from pointer to reference (since passing nullptr has no meaning for the underlying function). That's good. It also provides an overload TDC::GetClipBox () which returns the rectangle as the return value. That's even better. It is more user-friendly and composable, for less verbose and more robust C++ code (however, note that the implementation ignores the error code returned by ::GetClipBox — it should throw an exception!).

I guess we could use some formal guidelines on this. For now, these are my rules-of-thumb from the top of my head:

• Provide a thin wrapper as a member function of the same name for discoverability and 1-to-1 correspondence with the Windows API.
• Use OWL types for parameters where it makes sense, e.g. TRect instead of RECT, even in the 1-to-1 overload.
• Use exceptions rather than error codes.
• For getters, provide overloads that use the funtion return value to return the result, while throwing exceptions on errors.
• Consider std::optional if the function may or may not return something, and both outcomes are not exceptional (e.g. TRegKey::GetSubkey).
• For complex Windows functions with lots of multi-functionality and complex parameters, provide overloads, or secondary functions, and helper types that encapsulates the complexity.
  
  

For a really complex example of encapsulating the Windows API, see my latest work on TRegKey::GetSecurity. It has the thin wrapper overload complemented by a C++ style overload of the same name, TRegKey::GetSecurity (SECURITY_INFORMATION), returning the utility type TSecurityDescriptor, which in turn encapsulates the SECURITY_DESCRIPTOR type and associated functions in the Windows API.

[Vidar Hasfjord]

Also, see open ticket [feature-requests:#79] "Add TDC::GradientFill and TDC::AlphaBlend". It can be wrapped into this ticket. Go ahead! Take ownership! :-)

[Chris Kohlert]

did that. :) i will work on it :D

[Chris Kohlert]

Hi

The TSize-returning overload of GetTextExtentExPointI calls itself, rather than the raw overload,

:D :D i guess you are right, sorry i will fix that xD

GetAttributeHDC() and GetHDC() do the same thing, i didnt know when to use which. because GetAttributeHDC() is virtual and seem to be used with some printer-related functions, i thought i use it for getters rather than setters?!

the overload GetTextExtentPointI with the ignoring error-return in the patch is analog to other functions i found in the same spot for exact this purpose (ignoring the optional error result) - but i will change that to an exception in case of an error.

"Don't pass buffers as references to the first element"

thats why i used the original API-Type (LPSOMETHING) of that function.
i hoped that would make the purpose of using a buffer here clear :)
what would you do? use WORD glyphArray[] instead of LPWORD glyphArray?

"Having the member function also match the Windows API name and function parameters"

in case of the parameter-names thats not always a good idea because in the API often those names are cryptic and sometimes plain wrongly named (in this example the parameter that i named "glyphExtents" is original named "lpnDx" and the documentation is very misleading in what the array that it points to holds at the end).
i would vote that at least the parameter index should be the same as in the API for a thin layer.

another point in the wrapper is: is it a good idea to change the size of the vectors like i did above? should one do that this way - even for safety reasons - or is it too unexpected for the caller?

[Vidar Hasfjord]

GetAttributeHDC() and GetHDC() do the same thing, i didnt know when to use which.

I am confused, too. But I think it is important to get it right, because, as you point out, it is used in the printer preview functionality, in which a preview DC encapsulates a scaled DC (the previewed page).

the overload GetTextExtentPointI with the ignoring return in the patch is analog to other functions i found in the same spot for exact this purpose (ignoring the optional error result)

Aye. The TDC code is poor. I am studying it now. It lacks error handling in many places, const-correctness is poor (conceptual rather than actual, and sometimes inconsistent, as you noticed), and functions return the wrong return type sometimes also (bool instead of code). I will post some bug tickets soon. In Owlet, I may try a partial rewrite with consistent exception handling, which I will post later for feedback. Hopefully, we can set a better direction for the standard of this code.

thats why i used the original API-Type (LPSOMETHING) of that function. i hoped that would make the purpose of using a buffer here clear.

I was referring to this:

:::C++
inline uint32 TDC::GetGlyphIndices(LPCTSTR str, int count, WORD &glypharray, DWORD flags) const

Here, by conventional coding standards, glypharray is not a buffer. It is a reference to a single WORD. Treating it as the start of a buffer is undefined behaviour. Use a conventional buffer pointer and size.

:::C++
//
/// Returns size, throws on error.
//
inline auto TDC::GetGlyphIndices(LPCTSTR str, int count, WORD* glypharray, int glypharraySize, DWORD flags) const -> int;

in case of the parameter-names thats not always a good idea [to match the Windows API]

Fully agree. No Hungarian notation, please!

is it a good idea to change the size of the vectors like i did above?

Instinctively, I don't like out-parameters and in-out-parameters. Instead, I prefer to pass values and return values in pure functional style. If you need to return more than one value, create a structure (or use std::pair or std::tuple, if that is not too confusing).

That said, perhaps you shouldn't hard code vectors here at all. The user may want to use a different container type (C-array, std::array, dynamic allocation, etc.). Buffer pointer plus buffer size is flexible and conventional. If you want something neater, go for a generic contiguous range perhaps:

:::C++
template <class TContiguousRangeOfWord>
inline auto GetTextExtentExPointI(const TContiguousRangeOfWord& glyphs, int maxExtent) const -> std::pair<TSize, int>
{
  static_assert(std::is_same_v<WORD, std::remove_cv_t<std::remove_reference_t<decltype(*std::data(glyphs))>>>,
    "Element type must be WORD");
  auto extent = TSize{};
  auto fitCount = int{};
  const auto r = ::GetTextExtentExPointI(
    GetHDC(),
    const_cast<LPWORD>(std::data(glyphs)),
    static_cast<int>(std::size(glyphs)),
    maxExtent,
    &fitCount,
    nullptr,
    &extent);
  if (!r) throw TXGdi{IDS_GDIFAILURE, GetHDC()};
  return {extent, fitCount};
}

void test(TDC& dc)
{
  const WORD c1[3] = {1, 2, 3};
  const auto r1 = dc.GetTextExtentExPointI(c1, 42);

  const auto c2 = std::array<WORD, 3>{1, 2, 3};
  const auto r2 = dc.GetTextExtentExPointI(c2, 42);

  const auto c3 = std::vector<WORD>{1, 2, 3};
  const auto r3 = dc.GetTextExtentExPointI(c3, 42);

  const auto r4 = dc.GetTextExtentExPointI(
    std::initializer_list<WORD>{1, 2, 3}, 42);
}

For return values, you can, and should, return a vector where it makes sense.

:::C++
auto GetGlyphIndices(const tstring& s, uint flags = GGI_MARK_NONEXISTING_GLYPHS) const -> std::vector<WORD>
{
  const auto n = static_cast<int>(s.size());
  auto indices = std::vector<WORD>(n);
  const auto r = ::GetGlyphIndices(GetHDC(), s.c_str(), n, indices.data(), flags);
  if (r == GDI_ERROR) throw TXGdi{IDS_GDIFAILURE, GetHDC()};
  CHECK(r == n); // I assume all are converted, if successful.
  return indices;
}

In this case, there is only the vector to return, but for the more complex functions, you may need to package it into a struct with the other data returned. In any case, don't worry about returning vectors being inefficient; move semantics, C++17 copy elision and optimizers take care of that.

:::C++
struct TTextExtent : TSize
{
  std::vector<int> PartialExtents;
  auto GetFitCount() const { return static_cast<int>(PartialExtents.size()); }
};

template <class TContiguousRangeOfWord>
inline auto GetTextExtentExPointI(const TContiguousRangeOfWord& glyphs, int maxExtent) const -> TTextExtent
{
  static_assert(std::is_same_v<WORD, std::remove_cv_t<std::remove_reference_t<decltype(*std::data(glyphs))>>>,
    "Element type must be WORD");
  const auto glyphCount = static_cast<int>(std::size(glyphs));
  auto extent = TTextExtent{{}, std::vector<int>(glyphCount)};
  auto fitCount = int{};
  const auto r = ::GetTextExtentExPointI(
    GetHDC(),
    const_cast<LPWORD>(std::data(glyphs)),
    glyphCount,
    maxExtent,
    &fitCount,
    extent.PartialExtents.data(),
    &extent);
  if (!r) throw TXGdi{IDS_GDIFAILURE, GetHDC()};
  extent.PartialExtents.resize(fitCount);
  return extent;
}

[Chris Kohlert]

Okay, thanks, i try to keep all of that in mind :D

[Chris Kohlert]

template <class TContiguousRange>
inline auto GetTextExtentExPointI(const TContiguousRange& glyphs, 

... just whow. in that case good documentation is important because a user of this function couldnt see quickly that this is basically "some container of WORDs" ... xD

[Vidar Hasfjord]

good documentation is important

True. Although usage is simple in this case, templates can quickly become confusing (took me a while to get that static_assert right). But Doxygen is our friend. And in short order, we should assume the user is well versed in ranges and that C++20 is old-school. Time flies! :-)

[Vidar Hasfjord]

PS. I've fixed my code posted earlier, correcting naming ("bounding box" to "extent"), and added an example of a GetTextExtentExPointI implementation returning the partial extents vector as well in a utility structure.

[Vidar Hasfjord]

Thinking a bit about my implementation of GetTextExtentExPointI: While returning a vector is pretty efficient with modern C++, creating one over and over requires repeated dynamic allocation. So if you need to process a lot of strings, my posted solution isn't very efficient. However, it is pretty easy to fix this with move semantics and another optional parameter that lets the user pass in a vector for reuse. I think this should work, without requiring dynamic allocation in every call, except when required to grow the vector.

Like I said, we want lots of C++ in the 7 series! :-)

:::C++
struct TTextExtent : TSize
{
  std::vector<int> PartialExtents;
  auto GetFitCount() const { return static_cast<int>(PartialExtents.size()); }
};

template <class TContiguousRangeOfWord>
inline auto GetTextExtentExPointI(const TContiguousRangeOfWord& glyphs, 
  int maxExtent, TTextExtent&& extent = {}) const -> TTextExtent
{
  static_assert(std::is_same_v<WORD, std::remove_cv_t<std::remove_reference_t<decltype(*std::data(glyphs))>>>,
    "Element type must be WORD");
  const auto glyphCount = static_cast<int>(std::size(glyphs));
  extent.PartialExtents.resize(glyphCount);
  auto fitCount = int{};
  const auto r = ::GetTextExtentExPointI(
    GetHDC(),
    const_cast<LPWORD>(std::data(glyphs)),
    glyphCount,
    maxExtent,
    &fitCount,
    extent.PartialExtents.data(),
    &extent);
  if (!r) throw TXGdi{IDS_GDIFAILURE, GetHDC()};
  extent.PartialExtents.resize(fitCount);
  return extent;
}

void test(TDC& dc)
{
  const auto maxExtent = 42;

  const WORD c1[3] = {1, 2, 3};
  const auto r1 = dc.GetTextExtentExPointI(c1, maxExtent);

  const auto c2 = std::array<WORD, 3>{1, 2, 3};
  const auto r2 = dc.GetTextExtentExPointI(c2, maxExtent);

  const auto c3 = std::vector<WORD>{1, 2, 3};
  const auto r3 = dc.GetTextExtentExPointI(c3, maxExtent);

  const auto c4 = std::initializer_list<WORD>{1, 2, 3};
  const auto r4 = dc.GetTextExtentExPointI(c4, maxExtent);

  // Optimized vector use:
  // Provide a vector for reuse as an extra argument.
  // This must be passed with std::move, as to avoid copying.
  //
  const WORD c5[][3] = {{1, 2, 3}, {4, 5, 6}, {7, 8, 9}};
  auto r5 = TTextExtent{}; // Contains empty vector ready to be reused.
  for (auto& c : c5)
    r5 = dc.GetTextExtentExPointI(c, maxExtent, std::move(r5));
}

[Chris Kohlert]

okay, nice, but why is:
auto fitCount = int{};
so much better than
int fitCount;
? :D
(i need some moments to fully understand what you did there xD)
okay, i think this is a good solution, even if its on a c++ heavy side :D

[Vidar Hasfjord]

why is [auto style] so much better than [C style]

I am preparing for the day the latter is syntax error.

And it is more C++! :-)

(Seriously, check out Herb Sutter's talk on this; "Almost Always Auto style").

[Vidar Hasfjord]

okay, i think this is a good solution, even if its on a c++ heavy side :D

Good to hear! I just wrote a little test program to reassure myself that TTextExtent got the move semantics by default that I was expecting. Seeing C++ winding its way through function call, value return and repeated assignment without a copy in sight was lovely to behold. :-)

[Vidar Hasfjord]

why is auto i = int{}; so much better than int i;.

Let me elaborate upon that a little. It is about expressivity. Also, I have a simpler, safer and much more regular C++ language in the back of my head when I write code.

I was slightly flippant when I said I was preparing for the C style to become a syntax error. But actually that wouldn't be all bad. The convoluted declaration syntax and rules inherited from C complicates the C++ language standard a lot. Not that I have read it in detail, but the variable i exists in some kind of strange hinterland after the statement int i;. A variable name has been reserved, but no object yet exists.

:::C++
int i; TRACE("Declared i: " << i); // Undefined behaviour.
string s; TRACE("Declared s: " << s); // Just fine.

int i(); // Don't even try.
string s(); // Likewise.

int i{}; // Fine, but looks like that abomination above.
string s{}; // Likewise.

auto i = 0; // Fine. 0 is important.
while (...) ++i;

auto s = ""s; // Fine. Empty string is important.
while (...) s += ...;

auto i = int{}; // Alert! Default value. More to come!
ugly_get_int(&i); // Error code probably ignored.

auto s = string{}; // Alert! Default value. More to come!
ugly_get_string(&s); // Error code probably ignored.

const auto i = get_int(); // Super!
const auto s = get_string(); // Likewise.

// The neat regular C++ language I would really like:

#define def const auto // Define constant.
#define var auto // Define variable.

def i = get_int(); // Yohoo!
def s = get_string(); // Likewise.

def fn = [](...) { ... }; // Regular function syntax.
def mf = [this](...) { ... }; // Member function.
def lf = [&](...) { ... }; // Local function.

So, with my auto i = int{};, I brought an unfortunate situation in the code to your attention: I needed to reserve a variable before I could define its value (this is a problem with ugly APIs using out-parameters). Although less verbose, I didn't say auto i = 0;, because 0 is not important, and I didn't say int i = 0;, because that is superfluous type information (the compiler knows that 0 is an int), and I follow the rule that the important type information is on the right-hand side (AAA rule).

[Vidar Hasfjord]

To allow an initializer list to be used with GetTextExtentExPointI without explicit type qualification, i.e. GetTextExtentExPointI({...}, 42) rather than the rather long-winded GetTextExtentExPointI(std::initializer_list<WORD>{...}, 42), add this resolving overload, which simply forwards to the templated overload:

//
/// \overload
//
inline auto GetTextExtentExPointI(const std::initializer_list<WORD>& glyphs,
  int maxExtent, TTextExtent&& extent = {}) const -> TTextExtent
{
  return GetTextExtentExPointI<std::initializer_list<WORD>>(glyphs, maxExtent, std::move(extent));
}

void test(TDC& dc)
{
  const auto r = dc.GetTextExtentExPointI({1, 2, 3}, 42);
}

[Chris Kohlert]

maybe you should look into Rust a bit, i have the feeling you would like it ;)
https://doc.rust-lang.org/book/
your "def", "var" approach is pretty much how its done in Rust if i remember correctly :D

[Chris Kohlert]

can i wrap LPWORD + size into some type in c++ so that is is compatible to work as TContiguousRangeOfWord in your GetTextExtentExPointI example?
i mean - with out-of-the-box utilities? i cant find anything right now :D

[Chris Kohlert]

okay, i found it, initializer_list works even in this case just fine :D
https://godbolt.org/z/nM4cAk

[Vidar Hasfjord]

Rust is interesting. I have had it on my todo-list for a long time. I followed the D language development closely a while back — another system language contender. I need to check up on the progress there, also.

And interesting that initializer_list is useful to "range-ify" any contiguous sequence of element data. I have not thought of that. Thanks for sharing.

By the way, good to see another Compiler Explorer user here. That is an extraordinary cool tool. The author Godbolt does interesting presentations too.

[Vidar Hasfjord]

Regarding the optimisation we've explored here, it should be applied to other functions as well, of course, such as GetGlyphIndices, for which I offered a possible implementation earlier.

auto GetGlyphIndices(const tstring& s, uint flags = GGI_MARK_NONEXISTING_GLYPHS, 
  std::vector<WORD>&& indices = {}) const -> std::vector<WORD>
{
  const auto n = static_cast<int>(s.size());
  indices.resize(n);
  const auto r = ::GetGlyphIndices(GetHDC(), s.c_str(), n, indices.data(), flags);
  if (r == GDI_ERROR) throw TXGdi{IDS_GDIFAILURE, GetHDC()};
  CHECK(r == n); // I assume all are converted, if successful.
  return indices;
}

With this in place, it made me think that we now have a solution that may be able to beat the performance of Windows' own GetTextExtentPoint32 (encapsulated by TDC::GetTextExtentPoint) and GetTextExtentExPoint (not yet encapsulated). Internally, I assume that those functions are implemented by converting the string to a glyph array and then processing it by forwarding to one of GetTextExtentPointI or GetTextExtentExPointI. That, I guess, may very well involve dynamic allocation for the glyph array (unless Windows does some clever buffer reuse). We can eliminate that by doing the glyph conversion in a separate step while reusing our vectors:

using namespace std::string_literals;
const auto names = {_T("Jalmar"s), _T("Eldar"s), _T("Hilmar"s)};
auto glyphs = std::vector<WORD>{}; // Empty vector ready to be reused.
auto extent = TTextExtent{}; // Contains empty vector ready to be reused.
for (auto& n : names)
{
  glyphs = dc.GetGlyphIndices(n, GGI_MARK_NONEXISTING_GLYPHS, std::move(glyphs));
  extent = dc.GetTextExtentExPointI(glyphs, maxExtent, std::move(extent));
  // ...any processing here...
}

[Vidar Hasfjord]

By the way, do you know how in Doxygen you link to a specific function overload, when that overload is a function template?

[Chris Kohlert]

hmm, does

/// \overload template <class TContiguousRangeOfWord>
inline auto GetTextExtentExPointI(const TContiguousRangeOfWord& glyphs, 
  int maxExtent, TTextExtent&& extent = {}) const -> TTextExtent

not work? - i cant test it right now

[Chris Kohlert]

/// link: \link auto owl::TDC::GetTextExtentExPointI(const TContiguousRangeOfWord& glyphs, int  maxExtent) const -> TTextExtent \endlink

didnt work either :(