Re: [macppc] games/hyperrogue: fix build, but colors are off
On Tue, 15 Jan 2019 00:18:58 -0500
George Koehler wrote:
> On Mon, 14 Jan 2019 20:03:43 +0100
> Charlene Wendling wrote:
>
> > RCS file: patches/patch-shaders_cpp
>
> This patch fails because it has \n line endings (after I applied the
> patch in your email), but shaders.cpp has \r\n line endings. I fixed
> it by adding post-extract to Makefile:
>
> # Convert line endings before applying patch
> post-extract:
> cd ${WRKSRC} && perl -pi -e 's/\r$$//g' shaders.cpp
>
> The colors now look *much* better on my macppc machine.
I've seen it when using 'make update-patches', but has totally forgotten
to join the diff as an attachment for this reason, sorry.
> The tutorial segfaults and dumps core when I reach
> "Tiles and Tactics", press 2 to switch to Euclidean geometry, then
> press Enter to request the next slide. I didn't build with DEBUG=-g,
> so I have no useful backtrace.
Thanks. It's consistently reproducible on amd64 as well, with the same
exact steps. The only difference is that it produces a SIGBUS. I'll see
what can i do about it, but i'm totally unsure to be able to fix it.
Charlène.
> --
> George Koehler
>
Re: [macppc] games/hyperrogue: fix build, but colors are off
On Mon, 14 Jan 2019 23:20:46 +0100
Christian Weisgerber wrote:
> Charlene Wendling:
>
> > + void color2(color_t color, ld part) {
> > + unsigned char *c = (unsigned char*) (&color);
> > + GLfloat cols[4];
> > +- for(int i=0; i<4; i++) cols[i] = c[3-i] / 255.0 * part;
> > ++ for(int i=0; i<4; i++)
> > ++#if SDL_BYTEORDER == SDL_BIG_ENDIAN
> > ++cols[3-i] = c[3-i] / 255.0 * part;
> > ++#else
> > ++cols[i] = c[3-i] / 255.0 * part;
> > ++#endif
>
> My poor brain. I think this is correct but confusing. It's the
> ordering of c[] that is affected by endianness, not the ordering
> of cols[]. Also, I think you want { } for clarity.
>
> for(int i=0; i<4; i++) {
> #if SDL_BYTEORDER == SDL_BIG_ENDIAN
> cols[i] = c[i] / 255.0 * part;
> #else
> cols[i] = c[3-i] / 255.0 * part;
> #endif
> }
>
> Or maybe just extract the color channels with arithmetic?
>
> void color2(color_t color, ld part) {
> GLfloat cols[4];
> for(int i=0; i<4; i++)
> cols[0] = (color >> ((3-i) * 8) & 0xff) / 255.0 * part;
> ...
>
> The next function in shaders.cpp, colorClear(), is also wrong.
Using arithmetic also solves the issue, and in any case is less
confusing. colorClear() has been fixed as well. All colors are the same
on macppc and amd64(from packages and the patched ports).
I'm joining the diff as an attachement this time, due to different line
endings used in the code as George said.
Charlène.
> --
> Christian "naddy" Weisgerber
> na...@mips.inka.de
>
hyperrogue.diff
Description: Binary data
Re: [macppc] games/hyperrogue: fix build, but colors are off
On Mon, 14 Jan 2019 20:03:43 +0100
Charlene Wendling wrote:
> RCS file: patches/patch-shaders_cpp
This patch fails because it has \n line endings (after I applied the
patch in your email), but shaders.cpp has \r\n line endings. I fixed
it by adding post-extract to Makefile:
# Convert line endings before applying patch
post-extract:
cd ${WRKSRC} && perl -pi -e 's/\r$$//g' shaders.cpp
The colors now look *much* better on my macppc machine.
The tutorial segfaults and dumps core when I reach
"Tiles and Tactics", press 2 to switch to Euclidean geometry, then
press Enter to request the next slide. I didn't build with DEBUG=-g,
so I have no useful backtrace.
--
George Koehler
Re: [macppc] games/hyperrogue: fix build, but colors are off
Charlene Wendling:
> + void color2(color_t color, ld part) {
> + unsigned char *c = (unsigned char*) (&color);
> + GLfloat cols[4];
> +- for(int i=0; i<4; i++) cols[i] = c[3-i] / 255.0 * part;
> ++ for(int i=0; i<4; i++)
> ++#if SDL_BYTEORDER == SDL_BIG_ENDIAN
> ++cols[3-i] = c[3-i] / 255.0 * part;
> ++#else
> ++cols[i] = c[3-i] / 255.0 * part;
> ++#endif
My poor brain. I think this is correct but confusing. It's the
ordering of c[] that is affected by endianness, not the ordering
of cols[]. Also, I think you want { } for clarity.
for(int i=0; i<4; i++) {
#if SDL_BYTEORDER == SDL_BIG_ENDIAN
cols[i] = c[i] / 255.0 * part;
#else
cols[i] = c[3-i] / 255.0 * part;
#endif
}
Or maybe just extract the color channels with arithmetic?
void color2(color_t color, ld part) {
GLfloat cols[4];
for(int i=0; i<4; i++)
cols[0] = (color >> ((3-i) * 8) & 0xff) / 255.0 * part;
...
The next function in shaders.cpp, colorClear(), is also wrong.
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: [macppc] games/hyperrogue: fix build, but colors are off
On Mon, 14 Jan 2019 13:18:47 +0100
Christian Weisgerber wrote:
> George Koehler:
>
> > - unsigned char* c = (unsigned char*) &col; return c[i];
> > + unsigned char* c = (unsigned char*) &col;
> > +#ifdef __BIG_ENDIAN__
> > + return c[sizeof(col) - i];
> > +#else
> > + return c[i];
> > +#endif
> >}
>
> Off by one:
> return c[sizeof(col) - 1 - i];
>
> __BIG_ENDIAN__ is nonstandard. Normally, I'd suggest an
> endian.h-based check, but elsewhere in this code base there's
> already...
>
> #if SDL_BYTEORDER == SDL_BIG_ENDIAN
>
> ... so let's use that idiom.
>
> --
> Christian "naddy" Weisgerber
> na...@mips.inka.de
>
Hi,
Thanks to you two. It has fixed the textured mode, but not the OpenGL
one. I've found how to fix it as well, so there is an extra patch.
I tried to keep the #ifdef styling the same as upstream.
It builds and runs fine on macppc and amd64 [1].
Charlène.
[1] https://bsd.network/@julianaito/101416387255881269
Index: Makefile
===
RCS file: /cvs/ports/games/hyperrogue/Makefile,v
retrieving revision 1.8
diff -u -p -u -p -r1.8 Makefile
--- Makefile21 Nov 2018 01:58:38 - 1.8
+++ Makefile14 Jan 2019 18:15:53 -
@@ -2,6 +2,7 @@
COMMENT = roguelike game in a non-Euclidean world
CATEGORIES = games x11
+REVISION = 0
GH_ACCOUNT = zenorogue
GH_PROJECT = hyperrogue
@@ -17,7 +18,7 @@ WANTLIB += ${COMPILER_LIBCXX} GL GLEW SD
WANTLIB += c m png
# C++11
-COMPILER = base-clang ports-clang ports-gcc
+COMPILER = base-clang ports-gcc
BUILD_DEPENDS =${MODGNU_AUTOCONF_DEPENDS} \
${MODGNU_AUTOMAKE_DEPENDS}
Index: patches/patch-polygons_cpp
===
RCS file: patches/patch-polygons_cpp
diff -N patches/patch-polygons_cpp
--- /dev/null 1 Jan 1970 00:00:00 -
+++ patches/patch-polygons_cpp 14 Jan 2019 18:15:53 -
@@ -0,0 +1,19 @@
+$OpenBSD$
+Fix colors on big-endian machines in non-OpenGL mode.
+Index: polygons.cpp
+--- polygons.cpp.orig
polygons.cpp
+@@ -762,7 +762,12 @@ void fixMercator(bool tinf) {
+ }
+
+ unsigned char& part(color_t& col, int i) {
+- unsigned char* c = (unsigned char*) &col; return c[i];
++ unsigned char* c = (unsigned char*) &col;
++#if SDL_BYTEORDER == SDL_BIG_ENDIAN
++ return c[sizeof(col) - 1 - i];
++#else
++ return c[i];
++#endif
+ }
+
+ bool in_twopoint = false;
Index: patches/patch-shaders_cpp
===
RCS file: patches/patch-shaders_cpp
diff -N patches/patch-shaders_cpp
--- /dev/null 1 Jan 1970 00:00:00 -
+++ patches/patch-shaders_cpp 14 Jan 2019 18:15:53 -
@@ -0,0 +1,19 @@
+$OpenBSD$
+Fix colors on big-endian machines in OpenGL mode.
+Index: shaders.cpp
+--- shaders.cpp.orig
shaders.cpp
+@@ -341,7 +341,12 @@ void id_modelview() {
+ void color2(color_t color, ld part) {
+ unsigned char *c = (unsigned char*) (&color);
+ GLfloat cols[4];
+- for(int i=0; i<4; i++) cols[i] = c[3-i] / 255.0 * part;
++ for(int i=0; i<4; i++)
++#if SDL_BYTEORDER == SDL_BIG_ENDIAN
++cols[3-i] = c[3-i] / 255.0 * part;
++#else
++cols[i] = c[3-i] / 255.0 * part;
++#endif
+ #if CAP_SHADER
+ // glUniform4fv(current->uFog, 4, cols);
+ glUniform4f(current->uColor, cols[0], cols[1], cols[2], cols[3]);
Re: [macppc] games/hyperrogue: fix build, but colors are off
Christian Weisgerber wrote: > George Koehler: > > > - unsigned char* c = (unsigned char*) &col; return c[i]; > > + unsigned char* c = (unsigned char*) &col; > > +#ifdef __BIG_ENDIAN__ > > + return c[sizeof(col) - i]; > > +#else > > + return c[i]; > > +#endif > >} > > Off by one: > return c[sizeof(col) - 1 - i]; > > __BIG_ENDIAN__ is nonstandard. Normally, I'd suggest an endian.h-based > check, but elsewhere in this code base there's already... > > #if SDL_BYTEORDER == SDL_BIG_ENDIAN > > ... so let's use that idiom. Regarding +#ifdef __BIG_ENDIAN__ It isn't just non-standard, it is entirely wrong and absolutely breaks big-endian software since __BIG_ENDIAN__ is always defined as. One uses it with comparison...
Re: [macppc] games/hyperrogue: fix build, but colors are off
George Koehler: > - unsigned char* c = (unsigned char*) &col; return c[i]; > + unsigned char* c = (unsigned char*) &col; > +#ifdef __BIG_ENDIAN__ > + return c[sizeof(col) - i]; > +#else > + return c[i]; > +#endif >} Off by one: return c[sizeof(col) - 1 - i]; __BIG_ENDIAN__ is nonstandard. Normally, I'd suggest an endian.h-based check, but elsewhere in this code base there's already... #if SDL_BYTEORDER == SDL_BIG_ENDIAN ... so let's use that idiom. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: [macppc] games/hyperrogue: fix build, but colors are off
On Sun, 13 Jan 2019 17:34:04 +0100
Charlene Wendling wrote:
> Runtime is fine, but colors are off: the ice is red instead of
> blueish [1]. Anyone know where to start to look at?
There's an endian problem in polygons.cpp:part(). I tried to fix it
with the patch below, but it didn't work. My patch turned the ice
walls into invisible black tiles.
My PowerBook G4 with OpenBSD/macppc has 1G of RAM and uses more than
700M of swap to compile hyper.cpp, which includes most of the other
.cpp files. This takes about an hour, so I can't easily test changes
to the code.
=begin patches/patch-polygons_cpp
$OpenBSD$
Fix color on big-endian machines.
clang or gcc defines __BIG_ENDIAN__.
Index: polygons.cpp
--- polygons.cpp.orig
+++ polygons.cpp
@@ -762,7 +762,12 @@ void fixMercator(bool tinf) {
}
unsigned char& part(color_t& col, int i) {
- unsigned char* c = (unsigned char*) &col; return c[i];
+ unsigned char* c = (unsigned char*) &col;
+#ifdef __BIG_ENDIAN__
+ return c[sizeof(col) - i];
+#else
+ return c[i];
+#endif
}
bool in_twopoint = false;
=end
--
George Koehler
[macppc] games/hyperrogue: fix build, but colors are off
> http://build-failures.rhaalovely.net//powerpc/2019-01-01/games/hyperrogue.log
__multc3 is defined in compiler-rt, but is not built when building llvm.
So we need ports-gcc to build that port on macppc. I'm joining a diff
for that.
Runtime is fine, but colors are off: the ice is red instead of
blueish [1]. Anyone know where to start to look at?
Charlène.
[1] https://transfer.sh/AWemo/c21e4a8081b2e9b4.jpg
Index: Makefile
===
RCS file: /cvs/ports/games/hyperrogue/Makefile,v
retrieving revision 1.8
diff -u -p -u -p -r1.8 Makefile
--- Makefile21 Nov 2018 01:58:38 - 1.8
+++ Makefile13 Jan 2019 16:22:41 -
@@ -17,7 +17,7 @@ WANTLIB += ${COMPILER_LIBCXX} GL GLEW SD
WANTLIB += c m png
# C++11
-COMPILER = base-clang ports-clang ports-gcc
+COMPILER = base-clang ports-gcc
BUILD_DEPENDS =${MODGNU_AUTOCONF_DEPENDS} \
${MODGNU_AUTOMAKE_DEPENDS}
