Skip to content
This repository has been archived by the owner on Dec 2, 2019. It is now read-only.

Add subpixel API. #861

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add subpixel API. #861

wants to merge 4 commits into from

Conversation

itsuhane
Copy link

This PR tries to provide subpixel drawing API as discussed in #425.

In this PR, a new macro, NK_ENABLE_SUBPIXEL_API is proposed. User can turn-on subpixel API by defining this macro before ALL inclusion of nuklear.h.

When this macro is defined, the following changes are made inside nuklear:

  • All draw command structs (except nk_command_custom) will use float instead of short/unsigned short, use nk_vec2 instead off nk_vec2i.
  • All draw command routines (except nk_push_custom) will have a subpixel counterpart. For example, in addition to nk_stroke_polyline, nk_stroke_polyline_subpixel will be defined. In these subpixel version, there will be no narrowing from float to short.

No documentation is changed currently.

@itsuhane itsuhane marked this pull request as ready for review June 16, 2019 07:57
@dumblob
Copy link
Contributor

dumblob commented Jun 16, 2019

Thanks @itsuhane . An important question - it seems to me, that now it behaves differently than you proposed in #425:

As a proposal, there could be a global macro like NK_VERTEX_VALUE_TYPE, which defaults to short, but allow user to override with float to support subpixel drawing.
Current draw routines will keep the casting to make sure they don't broke any pixel-perfect drawing for UI elements. Meanwhile, they will have subpixel counterparts, nk_stroke_polyline_subpixel for example, for any user who want advanced control on their drawing.

use case NK_ENABLE_SUBPIXEL_API defined NK_ENABLE_SUBPIXEL_API NOT defined
old codebase all pixel-perfect calculations break (as nk_stroke_polyline behaves differently than originally) no change, everything works
new codebase works as expected (nk_stroke_polyline behaves the same as nk_stroke_polyline_subpixel) the old Nuklear behavior (i.e. internal conversion float -> short)

Thinking about this further we might actually afford making nk_stroke_polyline (and others) behave differently (i.e. not convert float to short internally) if and only if NK_ENABLE_SUBPIXEL_API (or rather NK_ENABLE_SUBPIXEL_PRECISION or alike as it has nothing to do with API) is defined (by default this macro won't be defined). So actually there shouldn't be any need for _subpixel() counterparts and the above table would look like this:

use case NK_ENABLE_SUBPIXEL_API defined NK_ENABLE_SUBPIXEL_API NOT defined
old codebase all pixel-perfect calculations break (as nk_stroke_polyline behaves differently than originally) no change, everything works
new codebase works as expected (nk_stroke_polyline doesn't do any float -> short conversions internally) the old Nuklear behavior (i.e. internal conversion float -> short)

Btw please also bump version accordingly 😉.

@itsuhane
Copy link
Author

itsuhane commented Jun 16, 2019

Thanks for your quick response!

Since we changed the internal representation for struct nk_command_*, some pixel-perfect version API must be changed accordingly. But their behavior was not changed: In the old codebase, we perform float-to-short conversions when creating these command structs. But before drawing, these short data are converted back to float, for example in line 1294 in nk_convert:

nuklear/src/nuklear_vertex.c

Lines 1291 to 1297 in bb327b9

int i;
const struct nk_command_polyline *p = (const struct nk_command_polyline*)cmd;
for (i = 0; i < p->point_count; ++i) {
struct nk_vec2 pnt = nk_vec2((float)p->points[i].x, (float)p->points[i].y);
nk_draw_list_path_line_to(&ctx->draw_list, pnt);
}
nk_draw_list_path_stroke(&ctx->draw_list, p->color, NK_STROKE_OPEN, p->line_thickness);

Effectively, the float-to-short conversion is just rounding-off the decimal parts. I think we can safely use float in commands without breaking the pixel-perfect drawing behavior.

Originally, I was thinking about having a macro named NK_VERTEX_VALUE_TYPE, and user can define it to float. However, there are three types involved in the command structs: nk_vec2i, short and unsigned short, if the user chooses float, unsigned float is neither meaningful nor legal in C. And in some compilers, unsigned float will be interpreted as unsigned int without giving an error. That's why I choose a different macro.

If we give up the conversion, all UI rendering will no-longer be pixel-perfect. I believe it will introduce artifacts. If there are artifacts, then we cannot easily remove the conversion. Two separate set of drawing API must co-exist. I will try subpixel UI drawing first and make some screenshot.

But in some sense, I agree to have only one set of implementation because my current PR contains massive code duplication. Can I create some macro ("templated C") to help elide the duplication ?

@dumblob
Copy link
Contributor

dumblob commented Jun 16, 2019

If we give up the conversion, all UI rendering will no-longer be pixel-perfect. I believe it will introduce artifacts.

This might be the case, but I think there is one problem which I'm reluctant to digest 😉. Namely, that Nuklear users won't have any clue which API (one of pixel-perfect and subpixel) in which case to use as they won't know how it behaves and how to combine the APIs. We must not introduce such problem at any cost.

I will try subpixel UI drawing first and make some screenshot.

This would be great - and once you're at it, try as many backends as you can to get some overview. This will be much appreciated.

Can I create some macro ("templated C") to help elide the duplication ?

Sure, just make some proposals a and we'll discuss it. Usually one comes with the best ideas when working with code, but not up front, so I won't comment technically on this until there is some practical demonstrations 😉.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants