2019-01-22 03:01 UTC

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0002336NetSurfRISC OS-specificpublic2016-02-16 14:45
ReporterSprow 
Assigned ToMichael Drake 
SeverityminorReproducibilityalways 
StatusclosedResolutionfixed 
Product Version3.3 
Target VersionFixed in Version3.4 
Summary0002336: Colours rendered incorrectly in 4k and 64k colour modes
DescriptionWhen choosing a 4k colour mode, or 64k colour mode, NetSurf's pages are rendered in the wrong colours (when Use OS/Use OS is selected in the 'Images' choices, as I suspect Tinct predates RISC OS Select getting 64k colour modes, and RISC OS 5 getting 4k + 64k colour modes).
Steps To ReproduceSwitch to a 4k or 64k colour mode.
Go to the default "about:welcome" page.
Additional InformationRisc PC, RISC OS 5.22.
TagsNo tags attached.
Fixed in CI build #3033
Reported in CI build #
URL of problem page
Attached Files
  • patch file icon buffer.patch (4,059 bytes) 2015-08-13 17:56 -
    --- riscos/bufferorg.c	2015-08-13 18:37:40.0 +0100
    +++ riscos/buffer.c	2015-08-13 18:37:40.0 +0100
    @@ -38,6 +38,11 @@
     #define BUFFER_EXCLUSIVE_USER_REDRAW "Only support pure user redraw (faster)"
     //#define BUFFER_EMULATE_32BPP "Redirect to a 32bpp sprite and plot with Tinct"
     
    +/** Absent from OSLib
    +*/
    +#define osspriteop_TYPEEXPANSION ((osspriteop_mode_word) 0xFu)
    +#define osspriteop_TYPE16BPP4K   ((osspriteop_mode_word) 0x10u)
    +
     static void ro_gui_buffer_free(void);
     
     
    @@ -151,28 +156,117 @@
     		static const ns_os_vdu_var_list vars = {
     			os_MODEVAR_LOG2_BPP,
     			{
    +				os_MODEVAR_MODE_FLAGS,
    +				os_MODEVAR_NCOLOUR,
     				os_MODEVAR_XEIG_FACTOR,
     				os_MODEVAR_YEIG_FACTOR,
     				os_VDUVAR_END_LIST
     			}
     		};
    -		int xeig, yeig;
    -		int vals[4];
    +		struct {
    +			int log2bpp;
    +			int flags;
    +			int ncolour;
    +			int xeig, yeig;
    +		} vals;
     		int type;
     
    -		error = xos_read_vdu_variables(PTR_OS_VDU_VAR_LIST(&vars), vals);
    +		error = xos_read_vdu_variables(PTR_OS_VDU_VAR_LIST(&vars), (int *)&vals);
     		if (error) {
     			LOG("Error reading mode properties '%s'", error->errmess);
     			ro_gui_buffer_free();
     			return;
     		}
     
    -		type = 1 + vals[0];
    -		xeig = vals[1];
    -		yeig = vals[2];
    -
    -		mode = (os_mode)((type << 27) | ((180 >> yeig) << 14) |
    -					((180 >> xeig) << 1) | 1);
    +		switch (vals.ncolour) {
    +		case 1:
    +		case 3:
    +		case 15:
    +		case 63:
    +		case 255:
    +			/* Paletted modes are pixel packing order agnostic */
    +			type = 1 + vals.log2bpp;
    +			mode = (os_mode)((type << osspriteop_TYPE_SHIFT) |
    +					osspriteop_NEW_STYLE |
    +					((180 >> vals.yeig) << osspriteop_YRES_SHIFT) |
    +					((180 >> vals.xeig) << osspriteop_XRES_SHIFT));
    +			break;
    +		case 4095:
    +			/* 16bpp 4k colours */
    +			type = osspriteop_TYPE16BPP4K;
    +			mode = (os_mode)((osspriteop_TYPEEXPANSION << osspriteop_TYPE_SHIFT) |
    +					osspriteop_NEW_STYLE |
    +					(vals.yeig << 6) |
    +					(vals.xeig << 4) |
    +					(type << 20) |
    +					(vals.flags & 0xFF00));
    +			break;
    +		case 65535:
    +			switch ((vals.flags & 0x3000) >> os_MODE_FLAG_DATA_FORMAT_SHIFT) {
    +			case os_MODE_FLAG_DATA_FORMAT_RGB:
    +				if (vals.flags & 0xC000) {
    +					/* Non VIDC packing order */
    +					if (vals.flags & os_MODE_FLAG_FULL_PALETTE)
    +						type = osspriteop_TYPE16BPP64K;
    +					else
    +						type = osspriteop_TYPE16BPP;
    +					mode = (os_mode)((osspriteop_TYPEEXPANSION << osspriteop_TYPE_SHIFT) |
    +							osspriteop_NEW_STYLE |
    +							(vals.yeig << 6) |
    +							(vals.xeig << 4) |
    +							(type << 20) |
    +							(vals.flags & 0xFF00));
    +				} else {
    +					/* VIDC packing order */
    +					if (vals.flags & os_MODE_FLAG_FULL_PALETTE)
    +						type = osspriteop_TYPE16BPP64K;
    +					else
    +						type = osspriteop_TYPE16BPP;
    +					mode = (os_mode)((type << osspriteop_TYPE_SHIFT) |
    +							osspriteop_NEW_STYLE |
    +							((180 >> vals.yeig) << osspriteop_YRES_SHIFT) |
    +							((180 >> vals.xeig) << osspriteop_XRES_SHIFT));
    +				}
    +				break;
    +			default:
    +				LOG("Unhandled 16bpp format from flags %d", vals.flags);
    +				ro_gui_buffer_free();
    +				return;
    +			}
    +			break;
    +		case -1:
    +			/* 16M colours */
    +			switch ((vals.flags & 0x3000) >> os_MODE_FLAG_DATA_FORMAT_SHIFT) {
    +			case os_MODE_FLAG_DATA_FORMAT_RGB:
    +				if (vals.flags & 0xC000) {
    +					/* Non VIDC packing order */
    +					type = osspriteop_TYPE32BPP;
    +					mode = (os_mode)((osspriteop_TYPEEXPANSION << osspriteop_TYPE_SHIFT) |
    +							osspriteop_NEW_STYLE |
    +							(vals.yeig << 6) |
    +							(vals.xeig << 4) |
    +							(type << 20) |
    +							(vals.flags & 0xFF00));
    +				} else {
    +					/* VIDC packing order */
    +					type = osspriteop_TYPE32BPP;
    +					mode = (os_mode)((type << osspriteop_TYPE_SHIFT) |
    +							osspriteop_NEW_STYLE |
    +							((180 >> vals.yeig) << osspriteop_YRES_SHIFT) |
    +							((180 >> vals.xeig) << osspriteop_XRES_SHIFT));
    +				}
    +				break;
    +			default:
    +				LOG("Unhandled 32bpp data format from flags %d", vals.flags);
    +				ro_gui_buffer_free();
    +				return;
    +			}
    +			break;
    +		default:
    +			LOG("Unhandled NCOLOUR value %d", vals.ncolour);
    +			ro_gui_buffer_free();
    +			return;
    +		}
     	}
     #endif
     
    
    patch file icon buffer.patch (4,059 bytes) 2015-08-13 17:56 +

-Relationships
+Relationships

-Notes
Sprow

~0000882

Sprow (reporter)

I've taken a quick look at the rendering code and have a reasonable idea of where the problem might be, but can't be 100% sure by inspection. I don't have any Linux system (nearest, cygwin in Windows), but wonder if there's a headless server somewhere with the environment already set up that I could just telnet/ssh into and type "make" having done local edits, then grab the executable via FTP to test. Rinse & repeat.
Rob Kendrick

~0000884

Rob Kendrick (administrator)

There is a guide on how to set up a virtual machine using VirtualBox (which is a free VM product for Windows and other platforms) on our wiki:

http://wiki.netsurf-browser.org/Documentation/BuildingForRISCOSQuickStart
Sprow

~0000889

Sprow (reporter)

Thanks Rob, that's a very useful page. I'd followed the "building NetSurf" link on the right of http://www.netsurf-browser.org/developers/ which led to http://source.netsurf-browser.org/netsurf.git/tree/Docs which seemed to be missing the instructions for RISC OS.

Have now installed VirtualBox and Debian 8 and produced a working !RunImage following those instructions, so it should be tractable to try out my fix now.

As an aside (this ticket is the wrong place) 'make TARGET=riscos package' resulted in a Messages file of 0 bytes, and Templates of 0 bytes. For now I just copied them out of an existing !NetSurf.
Sprow

~0000904

Sprow (reporter)

Here's a patch which fixes this issue. I've tested it in all 10 colour depths on RISC OS 5.22, and all 8 colour depths on RISC OS 4.02 (to check I've not broken them!).
Michael Drake

~0000905

Michael Drake (administrator)

Could you take a look at this Steve?
Michael Drake

~0001008

Michael Drake (administrator)

Applied your patch Sprow, thanks!
Vincent Sanders

~0001278

Vincent Sanders (administrator)

Confirmed fixed in 3.4 release
+Notes

-Issue History
Date Modified Username Field Change
2015-07-29 21:32 Sprow New Issue
2015-07-29 21:36 Sprow Note Added: 0000882
2015-08-11 10:26 Rob Kendrick Note Added: 0000884
2015-08-11 23:15 Sprow Note Added: 0000889
2015-08-13 17:56 Sprow File Added: buffer.patch
2015-08-13 17:57 Sprow Note Added: 0000904
2015-08-13 19:39 Michael Drake Note Added: 0000905
2015-08-13 19:39 Michael Drake Assigned To => Steve Fryatt
2015-08-13 19:39 Michael Drake Status new => acknowledged
2015-08-13 19:39 Michael Drake Steps to Reproduce Updated View Revisions
2015-10-31 19:04 Michael Drake Fixed in CI build # => 3033
2015-10-31 19:04 Michael Drake Note Added: 0001008
2015-10-31 19:04 Michael Drake Steps to Reproduce Updated View Revisions
2015-10-31 19:04 Michael Drake Assigned To Steve Fryatt => Michael Drake
2015-10-31 19:04 Michael Drake Status acknowledged => resolved
2015-10-31 19:04 Michael Drake Resolution open => fixed
2016-02-16 14:45 Vincent Sanders Note Added: 0001278
2016-02-16 14:45 Vincent Sanders Status resolved => closed
2016-02-16 14:45 Vincent Sanders Fixed in Version => 3.4
+Issue History