Probable bug in Dimensions

[Lord-Kamina]

Another yield from the text wrapping-up thing.

I was playing with text positioning on the main menu and getting some very inconsistent results. I know for a fact the X coordinates go from -0.5 to 0.5; I would think vertical coordinates would be the same. I wold expect if screenTop(0.0f) is the literal top of the screen, then screenTop(0.5f)should be vertical center of the screen, but it doesn't work that way.

I racked my head for a while trying to understand what the fuck was going on. I even built some old revisions going back to when I first started contributing to make sure it wasn't my fault, but even 1.1 has this problem. Eventually, I think I figured it out:

Now correct me if I'm wrong, but 0.1fshould represent 10% of the screen; that's the intended behavior, as far as I can tell. So, why does it work for horizontal but not vertical? Pretty simple actually, vertical coordinates do not adjust for aspect ratio.

To demonstrate, I replaced the logo with a simple white rectangle with a blue stroke.

The following picture is the result of changing the code in Game::drawLogo() to:

void Game::drawLogo() {
	m_logo.dimensions.fixedHeight(0.1f).left(-0.45f).screenTop(0.5f);
	m_logo.draw();
}

(ignore the fucked up menu positioning please, as that is what prompted my looking into this)

Now, take a look at this...

In my video_driver.hh I added target_height and target_AR, defined as follows:

const float targetWidth = 1366.0f; // One of the most common desktop resolutions in use today.
const float targetHeight = 768.0f; // One of the most common desktop resolutions in use today.
const float targetAR = targetWidth / targetHeight; // One of the most common desktop resolutions in use today.

Now, going back to the logo; if I do this:

void Game::drawLogo() {
	m_logo.dimensions.fixedHeight(0.1f).left(-0.45f).screenTop(0.5f / targetAR);
	m_logo.draw();
}

This is the result:

(I added the green guides in Photoshop for illustrating purposes)

[Lord-Kamina]

P.S. this is probably the cause of #10

[Tronic]

It is not a bug but aspect ratio thing. Units are same length vertically and horizontally, so on 16:9 display the height is only 0.56 units (and from top to middle half of that). Use middle/center as basis if you want center of the screen. That is specifically why it has separate accessors for each edge.

Trust me, my old code probably doesn't contain bugs every time you don't understand what it is doing.

[Lord-Kamina]

It is not a bug but aspect ratio thing. Units are same length vertically and horizontally, so on 16:9 display the height is only 0.56 units (and from top to middle half of that). Use middle/center as basis if you want center of the screen. That is specifically why it has separate accessors for each edge.

Trust me, my old code probably doesn't contain bugs every time you don't understand what it is doing.

Perhaps; as you say, it's not a bug but nonetheless, this is not documented and looking at some old issues, it has caused some trouble because it provides a non-intuitive interface to the code.

What I'm proposing would account for this and essentially allow for both horizontal and vertical coordinates in terms of percent (or fraction) of the screen in each direction (i.e. 1.0 is the entire screen; 0.1 is 10% of the screen, 0.01 is 1% of the screen).

While it's evidently possible to get correct positioning with the current code (I'm assuming most, if not all, the current positions on the code are correct) it's, as I say, unintuitive and requires much more trial-and-error to get right.

The reason I think this is important is because I'm doing a serious overhaul of the text coordinate system, to allow for text to wrap to multiple lines or ellipsize at arbritrary widths (or the eges of the screen) if there's not enough space; and to vary the amount of options that can be shown on a menu dynamically depending on the available screenspace.

[Tronic]

Then everything would be stretched, like cover images that should be square would become 16:9, and even text would appear distorted.

[Lord-Kamina]

Then everything would be stretched, like cover images that should be square would become 16:9, and even text would appear distorted. I suggest the project to remove your commit rights because all you do is fuck up things and don't listen to what you are told. I see you also switched to floats, although I specifically explained why doubles should be used in the other issue. For starters, do not 'fix' code that isn't broken.

No, because I'm not gonna commit anything until everything is adjusted so that current positioning and sizing is maintained.

Oh, and for the record, I wrote a bit of code to benchmark doubles against float operations and ran it on several x86 processors going from Kaby Lake, to Haswell, even all the way to Core2Quad and in most of them, floats were actually faster (even if not by much)

[Tronic]

You won't be able to change the code so that it would work the same with screen height always being 1.0. It would require a shitload of changes all around the code, and it would completely abolish the dynamic layout that Performous uses (try running singing screen in a window and resize the window to different shapes to see how UI elements are repositioned rather than stretched to fit).

[Lord-Kamina]

You won't be able to change the code so that it would work the same with screen height always being 1.0. It would require a shitload of changes all around the code, and it would completely abolish the dynamic layout that Performous uses (try running singing screen in a window and resize the window to different shapes to see how UI elements are repositioned rather than stretched to fit).

If that proves to be the case, I'll have wasted hours of my time and concede you were right; ok?

BTW; maybe my float v/s double testing routine can be improved but I wrote not to prove I'm right but because I was actually curious; here's what I came up with in case you're curious to try it:

#include <cmath>
#include <chrono>
#include <iostream>
#include <locale>
#include <random>
#include <string>
#include <vector>

using namespace std::chrono; 

namespace {
static std::default_random_engine e;
static std::uniform_real_distribution<double> distD(-79.654321, 137.654321);
        
	double get_double() {
		return distD(e);
	}
}

int main() {
	std::cout << "Preparing everything..." << std::endl;
	const size_t containerSize = 13337;
	const size_t iterations = 1000000;
	using dsec = std::chrono::duration<double>;
	using tps = std::chrono::time_point<high_resolution_clock, dsec>;

	std::vector<float> vFloat, floatResults;
	std::vector<double> vDouble, doubleResults;

	vFloat.reserve(containerSize);
	vDouble.reserve(containerSize);
	floatResults.reserve(containerSize);
	doubleResults.reserve(containerSize);

	for (size_t i = 0; i < containerSize; ++i) {
		double d = get_double();
		vDouble.push_back(d);
		vFloat.push_back(static_cast<float>(d));
	}

	std::string message("Will iterate "+std::to_string(iterations)+" times over "+std::to_string(containerSize)+" floats in both directions and alternate adding and substracting their square roots.");
	std::cout << message << std::endl;
	tps start, stop;
	dsec duration;
	std::string time;
	{
		start = high_resolution_clock::now();
		for (size_t iter = 0; iter < iterations; ++iter) {
			floatResults.clear();
			for (auto const& f: vFloat) {
				auto pos = (std::addressof(f) - std::addressof(vFloat.front()));
				float const& inverse = *(vFloat.rbegin() + pos);
				float result = std::sqrt(f);
				result += std::sqrt((iter % 2 == 0) ? inverse : (-inverse));
				floatResults.push_back(result);
			}
		}
		stop = high_resolution_clock::now();
		duration = stop - start; 
		time = std::string("Work took: "+std::to_string(duration.count())+" seconds.");
		std::cout << time << std::endl;
	}

	message = std::string("Will iterate "+std::to_string(iterations)+" times over "+std::to_string(containerSize)+" doubles in both directions and alternate adding and substracting their square roots.");
	std::cout << message << std::endl;
	{
		start = high_resolution_clock::now(); 
		for (size_t iter = 0; iter < iterations; ++iter) {
			doubleResults.clear();
			for (auto const& d: vDouble) {
				auto pos = (std::addressof(d) - std::addressof(vDouble.front()));
				double const& inverse = *(vDouble.rbegin() + pos);
				double result = std::sqrt(d);
				result += std::sqrt((iter % 2 == 0) ? inverse : (-inverse));
				doubleResults.push_back(result);
			}
		}
		stop = high_resolution_clock::now(); 
		duration = stop - start; 
		time = std::string("Work took: "+std::to_string(duration.count())+" seconds.");
		std::cout << time << std::endl;
	}
	std::cout << "Done." << std::endl;
	return 0;
};

[Tronic]

I'm glad you found the time to benchmark. Yes, they take some more memory and math library functions may use higher precision than they would otherwise (even though basic calculations +-*/ on CPU are just the same). However, how many such calculations do you think Performous is making per frame, or per second, or how many such variables there are? Then calculate how many nanoseconds of difference that would make for frame rendering time (compared to ~16 ms that a frame gets to display). This is why I told you that doubles don't make any difference for individual variables, only in large arrays.

[Lord-Kamina]

I'm glad you found the time to benchmark. Yes, they take some more memory and math library functions may use higher precision than they would otherwise (even though basic calculations +-*/ on CPU are just the same). However, how many such calculations do you think Performous is making per frame, or per second, or how many such variables there are? Then calculate how many nanoseconds of difference that would make for frame rendering time (compared to ~16 ms that a frame gets to display). This is why I told you that doubles don't make any difference for individual variables, only in large arrays.

My reasoning is that if for all intents and purposes performance is the same, with floats we'll still be using less memory; and if we actually do get an increase in performance (small as it may be), then all the better.

[Tronic]

If that proves to be the case, I'll have wasted hours of my time and concede you were right; ok?

Based on prior experience, I think you'll break a shitload of things but claim that everything is working, and make a PR and someone will merge it without looking too carefully.

My reasoning is that if for all intents and purposes performance is the same, with floats we'll still be using less memory; and if we actually do get an increase in performance (small as it may be), then all the better.

What I am saying is that there is no noticeable difference in performance or memory usage, but that code clarity is a far more important factor, and also that unnecessary changes should be avoided as they may introduce new bugs. For code clarity, it is clearly easier to write the numeric values as doubles, without an f, and generally all programs should be using double whenever it is not necessary to use other floating-point types (which pretty much only concerns arrays).

I've had enough with this thread, got work to do, and won't respond any more. @Baklap4 will probably weigh in if there is something more to be said.