Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: small & stupid api changes: radians, vectors, colors  (Read 15180 times)

0 Members and 1 Guest are viewing this topic.

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6286
  • Thor Developer
    • View Profile
    • Bromeon
Re: small & stupid api changes: radians, vectors, colors
« Reply #15 on: September 23, 2014, 06:54:39 am »
Or maybe there is another straightforward way of writing this code that I'm not aware of?
Global function:
template <typename T>
void setAlpha(T& object, sf::Uint8 alpha)
{
    sf::Color color = object.getColor();
    color.a = alpha;
    object.setColor(color);
}

Works now with any class providing access to color, not just sf::Text.
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

cepro

  • Newbie
  • *
  • Posts: 12
    • View Profile
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #16 on: September 23, 2014, 12:51:39 pm »
Thank you @dabbertorres and @Nexus, well that looks a bit cleaner :) It will always need to copy the object's color thought but hey it does the job and it's clean. :)

Tank

  • SFML Team
  • Hero Member
  • *****
  • Posts: 1486
    • View Profile
    • Blog
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #17 on: September 26, 2014, 07:19:43 am »
I like 1. but in a way Nexus suggested. It's a very clean approach.

I'm not sure about 2. Like others said, it would only support a subset of possible structures. If the source uses a different naming convention, it wouldn't work already. Thus the constructor isn't generic.

3. sounds like a very good idea, as using single values for colors is very common.  I wouldn't use named constructors (static functions) here, but only a constructor that takes sf::Uint32, representing RGBA.

achpile

  • Full Member
  • ***
  • Posts: 231
    • View Profile
    • Achpile's homepage
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #18 on: September 26, 2014, 01:27:49 pm »
May be it will be interesting to implement sf::Angle class with methods, returning Deg value, Rad value and sf::Vector2f value. for example

inline sf::Vector2f sf::Angle::toVector(float len = 1.0f) {
    return sf::Vector2f(cos(m_angle), sin(m_angle)) * len;
};
 

But it will be a class with only 1 member (m_angle), so it's better to make it just a functions. And I think everyone can implement what he wants. And now I don't even know why I've written this reply :D

FRex

  • Hero Member
  • *****
  • Posts: 1845
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #19 on: October 10, 2014, 05:49:16 pm »
Since this thread died here's a patch:
« Last Edit: August 31, 2015, 10:23:02 pm by FRex »
Back to C++ gamedev with SFML in May 2023

wintertime

  • Sr. Member
  • ****
  • Posts: 255
    • View Profile
Re: small & stupid api changes: radians, vectors, colors
« Reply #20 on: October 12, 2014, 01:29:49 pm »
The problem I see with doing that is: there is no standardized way of packing rgba into an int.

FRex

  • Hero Member
  • *****
  • Posts: 1845
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #21 on: October 12, 2014, 03:55:25 pm »
Let's be serious now - SFML uses R8G8B8A8 exclusively, not ARGB or BGRA or any other format so it's natural to expect 4 bytes packed into int in that order.
By the logic you use someone would say that RGBA is not that standard channel order either and say it's not intuitive to have the Color(Uint8 r, Uint8 g, Uint8 b, Uint8 a) constructor too (because he has to read the docs/names of arguments to know the order, and he expected it to be ARGB).
Besides RGBA is (arguably) the most intuitive/popular order (so even newbs will get it) and is basically same as HTML/CSS hex with two more digits at end (for alpha).
This also doesn't get in the way for any current users because it's just a shortcut, there is no danger someone will accidentally pass unsigned there and be confused what happened, they need to read the doc to know it's there before using it and then, seeing the docs comment, they will know it goes in RGBA order.
Back to C++ gamedev with SFML in May 2023

wintertime

  • Sr. Member
  • ****
  • Posts: 255
    • View Profile
Re: small & stupid api changes: radians, vectors, colors
« Reply #22 on: October 12, 2014, 10:14:51 pm »
Even if someone uses r,g,b,a it can still result in a different byte order depending on the endianness:
union {
  Uint32 rgba;
  sf::Color sfcolor;
} mycolor;

mycolor mc;
mc.sfcolor=sf::Color::Green;
sf::Color color(mycolor.rgba);
if(color==sf::Color::Blue)
  std::cout << "argh" << endl;
 

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32504
    • View Profile
    • SFML's website
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #23 on: October 12, 2014, 10:56:49 pm »
It can result in a different byte order in memory, but we don't care. RGBA usually refers to the "logical" byte order (R is the MSB, then G, then B, and A is the LSB).

sf::Uint32 color = 0x11223344;
sf::Uint8 r = (color >> 24) & 0xFF; // 0x11
sf::Uint8 g = (color >> 16) & 0xFF; // 0x22
sf::Uint8 b = (color >>  8) & 0xFF; // 0x33
sf::Uint8 a = (color >>  0) & 0xFF; // 0x44

... which doesn't depend on the machine's endianness.
Laurent Gomila - SFML developer

FRex

  • Hero Member
  • *****
  • Posts: 1845
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #24 on: October 12, 2014, 11:16:09 pm »
You declared unnamed global union named mycolor, not a new union type called mycolor.
This is one of the reasons it won't compile.
A new union type named mycolor would be:
union mycolor{
  Uint32 rgba;
  sf::Color sfcolor;
};
Also, this is quite an artificial problem, no wants to put sf::Color and sf::Uint32 into an union like you shown, we only intend to add the new constructor.
And actually, an union containing sf::Color will not compile, in C++98 it has a constructor which is forbidden, in C++11 (which relaxed the rules) it has non trivial constructor so it won't compile either.

IF you mean that this byte packing doesn't work on big endian then you are not right either, look:
#include <cstdio>

#define rf 0xff000000
#define gf 0x00ff0000
#define bf 0x0000ff00
#define af 0x000000ff

typedef unsigned char u8;
typedef unsigned u32;

class Color
{
public:

    Color(u8 r, u8 g, u8 b, u8 a = 255):
    r(r), g(g), b(b), a(a) {}
   
    Color(u32 col):
    r((col & rf) >> 24), g((col & gf) >> 16), b((col & bf) >> 8), a(col & af) {}

    u8 r, g, b, a;
};

int lilend()
{
    int i = 1;
    return (*(char *)&i) == 1;
}

void printcol(Color c)
{
    std::printf("r = %x, g = %x, b = %x, a = %x\n", c.r, c.g, c.b, c.a);
}

int main()
{
    std::printf("endian lil = %d\n", lilend());
    std::printf("sizeof u32 = %d sizeof u8 = %d\n", sizeof(u32), sizeof(u8));
    Color c(0x00ff00ff);
    std::printf("next line is 0, ff, 0, ff\n");
    printcol(c);
    c.r = 255;
    c.g = 0;
    std::printf("next line is ff, 0, 0, ff\n");
    printcol(c);
}

I ran this code on few machines (since I can't get full SFML there, we use equivalent code like one we intend to use in sf::Color).

This is from an old machine running 32 bit (at least I think so because size of pointer is 4) Sun-OS 5.10 on big endian CPU (it's sparc I think, not sure now, but definitely big endian...).
endian lil = 0
sizeof u32 = 4 sizeof u8 = 1
next line is 0, ff, 0, ff
r = 0, g = ff, b = 0, a = ff
next line is ff, 0, 0, ff
r = ff, g = 0, b = 0, a = ff

This is from my local machine, which is obviously standard Intel CPU, 64 bit capable but running 32 bit Fedora 20 with little endian.
endian lil = 1
sizeof u32 = 4 sizeof u8 = 1
next line is 0, ff, 0, ff
r = 0, g = ff, b = 0, a = ff
next line is ff, 0, 0, ff
r = ff, g = 0, b = 0, a = ff

I by mistake ran it on a multicore FreeBSD on AMD64 (also little endian) and result was same as on my machine.

As you see, endian and bitness doesn't affect this kind of thing, the constructor itself works as intended, we can use the color normally, etc. and unions are non issue, since sf::Color can't be put in one anyway.
« Last Edit: October 12, 2014, 11:28:24 pm by FRex »
Back to C++ gamedev with SFML in May 2023

wintertime

  • Sr. Member
  • ****
  • Posts: 255
    • View Profile
Re: small & stupid api changes: radians, vectors, colors
« Reply #25 on: October 12, 2014, 11:49:11 pm »
Using your new contructor with a hex literal will obviously work like you want.

Doing an unsafe pointercast or using such a union, setting bytes first, then as a misoptimization reading the whole as an Uint32 would reverse it on little endian. And sure one should not do such as it depends on undefined behaviour, it was just a tiny example to demontrate its not needed to willingly use abgr and still get data in reverse order, to show it needs good documentation.

PS: Yes, I should not have shortened the example by replacing the struct I had there first with sf::Color and possibly have tried to compile it beforehand. :-[

FRex

  • Hero Member
  • *****
  • Posts: 1845
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #26 on: October 13, 2014, 12:07:31 am »
Quote
using such a union
As I said above, you can't use such an union with sf::Color.

What does the fact someone can get wrong result if they (mis)use an union or reinterpret cast on one endian or the other have to do with this feature and this implementation of it?
As Laurent said, by RGBA order we mean it in "logical"/"human" order, the "first" byte is the first one you write when typing the number by hand (so the one under 0xff000000 mask regardless of the fact if it's last or first in RAM) and so on.
Back to C++ gamedev with SFML in May 2023

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32504
    • View Profile
    • SFML's website
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #27 on: October 13, 2014, 07:57:18 am »
Please can we stop this useless discussion? The feature is perfectly valid, has been accepted and is just waiting to be added to the code...
Laurent Gomila - SFML developer

FRex

  • Hero Member
  • *****
  • Posts: 1845
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #28 on: October 13, 2014, 09:48:50 am »
Go ahead then, copy the patch I gave into a file and git am it.
We can add the color -> unsigned member function later if needed and it's less important than this constructor anyway.
Back to C++ gamedev with SFML in May 2023

Tank

  • SFML Team
  • Hero Member
  • *****
  • Posts: 1486
    • View Profile
    • Blog
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #29 on: October 13, 2014, 12:04:01 pm »
Thanks for the patch. Maybe it's possible to send in a GitHub pull request next time, as that eases the process a bit. For now, here's the PR: https://github.com/SFML/SFML/pull/722