StrobeAPI Implementation #353
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,8 @@ GNU General Public License for more details. | |
| #include "particledef.h" | ||
| #include "entity_types.h" | ||
|
|
||
| #include "strobe/r_strobe_core.h" | ||
|
|
||
| #define IsLiquidContents( cnt ) ( cnt == CONTENTS_WATER || cnt == CONTENTS_SLIME || cnt == CONTENTS_LAVA ) | ||
|
|
||
| msurface_t *r_debug_surface; | ||
|
|
@@ -1326,99 +1328,21 @@ void R_RenderFrame( const ref_params_t *fd, qboolean drawWorld ) | |
| GL_BackendEndFrame(); | ||
| } | ||
|
|
||
| /* | ||
| =============== | ||
| R_EndFrame | ||
| =============== | ||
| */ | ||
| void R_EndFrame( void ) | ||
| { | ||
| #ifdef STROBE_ENABLED | ||
| Strobe_Invoker((void**)(&STROBE_CORE), STROBE_CORE_EXPORTEDFUNC_constructor, STROBE_CORE_EXPORTEDFUNC_main, STROBE_CORE_EXPORTEDFUNC_destructor); | ||
|
There was a problem hiding this comment. Now it looks more ugly. Can you just look to other parts of engine and do similar? There was a problem hiding this comment. Yeah I know it is very ugly. But I do not think that this can be not ugly with the object orient structure intact. It is C after all. I have thought using GObject or something like that but decided not to use because I want this strobe implementation to be self-contained. There was a problem hiding this comment. In what parts of the engine for instance ? There was a problem hiding this comment.
You will not find any feature, that's implementation is smudged over the whole engine. Only in such way engine development feels friendly and comfortable. Thanks to Carmack for it. Look into Quake engine, this is fully dervied from it. Some Quake-derived engines still use this, like QFusion, that's why I found QFusion code is beatiful. I don't know why you think that you need OOP here. Yes, you may simulate some aspects from it, like encapsulation(example above) or polymorphism(see in backends. :)). And that's enough for everything in game engine. There was a problem hiding this comment. Don't think that it's too hard to send patches for such small and young(compared to original Xash3D) project. I just want to keep single and simple way to work with engine source code. Otherwise it will go fast into the pile of garbage of too different code, like Darkplaces. There was a problem hiding this comment. But the concept here is different. This way a developer can develop their own strobe implementation without altering the core implementation and run it along with different implementations. Also with this way, even if STROBE_ENABLED is defined, strobe will not allocate unnecessary space in memory unless user sets r_strobe to non zero (for core implementation). https://vgy.me/xnhxKP.png . I think if oop is not used here, either "strobe" will be all over the place or it will suck really hard memory-wise. Also maintenancing new implementations will be much harder. I think it is a fair trade to use oop here. There was a problem hiding this comment. This a generic game engine in first place. If someone want to develop their strobe algorithm, they can implement it in client library. There was a problem hiding this comment. Good idea! Long term (eventually) this could be a generic strobe library. Problem is that strobing requires very low-level synchronization and requires a certain amount of very tight 2-way integration as fuzun's diagram explains -- https://vgy.me/xnhxKP.png -- very good points. The strobe API should ideally have its own independently configurable features (e.g. black frame ratios) and maybe 'hint' to the rest of the game of its preferred patterns (e.g. a "biasing" to 72fps during 144Hz operation, or 45fps/60fps/90fps modes for various strobe patterns of 180Hz strobing -- the game has to change its framerate to adapt to the preferred strobe algorithm -- necessitiating a 2-way influence between the strobe API and the game engine, to maximize smoothness and minimize input lag, etc) It's a code architectural challenge -- how to do it in a clean way, is a super-big-challenge. One can only do their best to self-contain the strobing, but will require quite a bunch of low-level hooks sprinkled throughout any game engine or emulators. Theoretical independent workarounds can be done such as adding detection of VBlanks (e.g. Direct3D9 RasterStatus.InVBlank but that is not always reliable) but it can be very hard to detect if a framebuffer flip (Present) made it to the correct refresh cycle. Different engines have different ways of framepacing, and it's almost impossible to fully decouple a strobe library from the game engine behaviour, unless you're doing it at a super low level (e.g. attempting to do software strobing directly in NVIDIA graphics drivers, for example) -- and doing it at the driver levels will have more input lag than doing it correctly at the game engine level. BTW, one thing I noticed on TN LCDs. 60Hz strobing at 180Hz has better color quality than 60Hz strobing at 120Hz. This is because the LCD 6-bit FRC often alternates the temporal dithering between even/odd refresh cycles so you get a color-depth reduction with software BFI on a 2-refresh-pattern on certain LCDs. So by adding a 3-refresh-cycle rotation (ON-OFF-OFF) or (ON-ON-OFF) rather than a 2-refresh-cycle rotation (ON-OFF) you gain back the full color depth of TN LCD 6-bit FRC during 60Hz software strobing. Anyway, a very interesting observation I've found. Yet another great pro of keeping flexibility for customizable number of BFI counts in any strobe API. Nontheless I agree: Keep the amount of code outside of strobe API as small as possible. There'll be necessarily hooks (especially correct VBI timing, and correct hinting or control of desired frame pacing / desired frametimes that fits the strobe API's current strobe algorithm) - But yep -- it's a good idea to minimize amount of code outside the strobe module the absolute bare minimum where possible, and blackbox as much stuff as possible into the strobe library (where it doesn't impact lag) -- e.g. desired frametimes between unique frames -- to target a strobe algorithm that requires 2-vblank, 3-vblank, custom VRR (e.g. X number of different-length VRR refresh cycles for variable-persistence software BFI, etc), or whatnot. There was a problem hiding this comment. P.S. I see source code license says "This project uses Quake's C code style convention." If there's a disagreement about using OOP or not .... how hard is it to use a client library approach? Or some other simplification tricks. Keep the OOP inside the library and simply use library calls from the favourite C project. It may mean additional work in the strobe module though, to make it simpler on various game engine authors themselves. Sometimes larger amounts of prettier code is preferable over smaller amounts of uglier code -- even from a maintainability perspective. Beauty is in the eye of the beholder... Sometimes you prefer to actually consume (a small bit) of memory to simplify maintenance on all sides. Dummy strobe wrapper could also automatically be used (few bytes of memory to cover dummy calls) when strobe is disabled at compile time. That way, you can remove those #ifdefs and constructors/destructor parameters. Who cares about 100 bytes of memory by automatically using the dummywrapper strobe whenever strobe is disabled? I'd gladly waste 100 bytes to remove those #ifdefs possibly -- it is mostly stuff called once per frame render, so that almost-zero extra CPU cycles occuring only 60 times a second, wouldn't matter, if it's an immediate return anyway from a stub-out. Even with C, there are various tricks like header files containing #define/#undef to dummy-out the calls (low-memory do-nothing version of strobe module, when strobe disabled) without sprinking #ifdefs throughout code. Smart compilers will probably actually compile it to nothingness anyway. It's all a programmer's decision on tradeoffs on memory-and-code-readability. Easing maintainability of strobe shouldn't degrade readability/maintainability on the engine side -- and that includes sometimes wasting 100 bytes of memory to remove hundreds of #ifdefs that are inside *.c instead of *.h -- tough decisions I know -- but... In some cases one can use a global inside the strobe module to contain more of the OOP stuff to be contained only within the strobe module -- basically a defacto non-OOP wrapper on OOP code for those engines that strictly want to stay non-OOP. This could also additionally be the technique you use to dummy-out too (e.g. assign dummy functions in strobe-disable, assign working functions in strobe-enable). Since usually only one strobe module active, certain things just simply be a global variable for a very good legitimate reason. Certainly, global is often discouraged, but in this case, it might be appropriate win-win compromise if agreed -- if it hides a little OOP overhead from the non-strobe code in a strongly traditional Quake / ANSI C engine (without needing to do much refactoring) and also hits two birds with one stone (e.g. makes it easier to dummy-out during strobe-disable, making it possible to remove #ifdef clutter). Looks like plenty of halfway-meeting "compromises" that could keep both of you happy? I only briefly took a look, and I may have missed a lot of blockers (or code architecture decisions) that prevent most/all my suggestions -- just throwing ideas out on the table to help both of ya out... Just brainstorming ways to make everyone happy :) There was a problem hiding this comment. @mdrejhon - Thank you for sharing opinion. I want to clarify a few points. "it's a good idea to minimize amount of code outside the strobe module the absolute bare minimum where possible, and blackbox as much stuff as possible into the strobe library" this was the main point of making this built on object oriented structure so that other implementations will be able to made with ease and with good memory handling. But it also stands out among the other Xash3d source files (structure is fundementally different as @a1batross have pointed out) and because it is hardcoded OOP in C. With this way, a different implementation can be made really easily. I think that strobing is a very complex thing and the "core" implementation itself is not sufficient. For example it does not support varied positive phase shifting (think as PWM signal, not the other phase). Look at the template files as an example (They have a few errors though that I will correct), they contain the basic structure for making a new strobing implementation. It is very easy to make because Strobe API provides all necessary backend functions. Even the debug script is generated by that. The only thing that needs to be done is implementing the algorithm and updating protected members of the abstract base class/structure: Strobe-api through the new code. And including the new .h file and calling Strobe_Invoke function each frame and/or calling (own) debughandler function for drawing the debug script. And yeah beyond this point it is all about decisions. Since it is C, I do not think that it will get much better than this. If it will have less OOP, maintenance and implementation will be harder. If it will have more OOP, it will be more bloated. There is a trade. There was a problem hiding this comment. Okay, I understood. You only need to set up your IDE to match Unkle Mike's code style(a space after '(' and a space before ')', excluding repeated brackets). If your IDE doesn't support it, there is clang-format settings at the root of repository. And then I will merge it ASAP. |
||
| #else | ||
| // flush any remaining 2D bits | ||
| R_Set2DMode(false); | ||
| #endif | ||
|
|
||
|
|
||
| #ifdef XASH_SDL | ||
| SDL_GL_SwapWindow( host.hWnd ); | ||
| #elif defined __ANDROID__ // For direct android backend | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well... now it looks like something from Windows API...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Macros can be extended to make this more pretty but there are already a lot of them.