Re: KolibriOS Port for Netsurf

On 14/05/16 03:06, Ashish Gupta wrote:

> My nick on #netsurf on Freenode : ashmew2

Hi, Ashish!

Thanks for doing a NetSurf port for KolibriOS.

> This is the first of a few patches / additions to the netsurf code
> base to fully support the KolibriOS port and eventually merge to the
> Jenkins CI.

> With that said, could someone review the libnsfb surface for KolibriOS
> ? I've tried to stick to the Netsurf code style guide as much as
> possible but also drew some inspiration from the already existing
> surfaces. Once this is finalized, I'll patch the libnsfb Makefile as
> well.

OK, I will comment in line below.

I'm more familiar with the libnsfb plot code, rather than the surface
handlers, so perhaps it would be worth Vince having a look. It looked
fine to me.

> I've attached the kolibri.c file to this email. The same file is also
> available on github at the primary development repository[1] . If you
> are interested in the progress, the forum thread on KolibriOS Board[2]
> will help you understand more. If it is more convenient for you, you
> can also comment on github directly (Let me know, so I can grant you
> rights as required).

Daniel, can we get branch commit access for Ashish on the NetSurf git?

> /*
> * Copyright 2016 Nina Kalinina <ninacarrot@ya.ru>
> * Copyright 2016 Ashish Gupta <ashmew2@gmail.com>
> *
> * This file is a part of libnsfb, http://www.netsurf-browser.org/
> * Licenced under the MIT License,
> * http://www.opensource.org/licenses/mit-license.php
> */
>
> #include <stdbool.h>
> #include <stdlib.h>
>
> #include "libnsfb.h"
> #include "libnsfb_event.h"
> #include "libnsfb_plot.h"
> #include "libnsfb_plot_util.h"
>
> #include "nsfb.h"
> #include "surface.h"
> #include "palette.h"
> #include "plot.h"
> #include "cursor.h"
>
> /* Define codes for KolibriOS' events */
> #define EVENT_REDRAW 0x00000001
> #define EVENT_KEY 0x00000002
> #define EVENT_BUTTON 0x00000004
> #define EVENT_END_REQUEST 0x00000008
> #define EVENT_DESKTOP_BACK_DRAW 0x00000010
> #define EVENT_MOUSE_CHANGE 0x00000020
> #define EVENT_IPC 0x00000040

NetSurf style is usually:

#define EVENT_REDRAW (1 << 0)
#define EVENT_KEY (1 << 1)
#define EVENT_BUTTON (1 << 2)
#define EVENT_END_REQUEST (1 << 3)
#define EVENT_DESKTOP_BACK_DRAW (1 << 4)
#define EVENT_MOUSE_CHANGE (1 << 5)
#define EVENT_IPC (1 << 6)

I'd add defines for the other bits which are set in the code later, too:

#define WHATEVER_0 (1 << 7)
#define WHATEVER_1 (1 << 30)
#define WHATEVER_2 (1 << 31)


> /* Pixel array which we need to pass around. */
> unsigned char * pixels;
> unsigned previous_mouse_position, previous_mouse_buttons;
>
> int kolibri_get_button_id(void)
> {
> uint16_t __ret;
> __asm__ __volatile__ (
> "int $0x40"
> :"=a"(__ret)
> :"0"(17));
>
> if((__ret & 0xFF) == 0)
> return (__ret >> 8) & 0xFF;
> else
> return -1;
> }

NetSurf style is to have a space between keyword and '('., and to
use {...} blocks even for single lines.

if ((__ret & 0xFF) == 0) {
return (__ret >> 8) & 0xFF;
} else {
return -1;
}

I am not personally too bothered about style in frontend-specific code,
so its up to you if you want to change this sort of thing.

> /*This is for setting flags for mcall40 for events read by a window*/
> kolibri_set_wanted_events(EVENT_REDRAW |
> EVENT_KEY |
> EVENT_BUTTON |
> EVENT_MOUSE_CHANGE |
> (1 << 30) | (1 << 31) | (1 << 7));

These are the bits that could be #defined.


> static int kolibri_surface_claim(nsfb_t *nsfb, nsfb_bbox_t *box)
> {
> /* TODO: Convert to full function from stub */
>
> /*
> if ((cursor != NULL) &&
> (cursor->plotted == true) &&
> (nsfb_plot_bbox_intersect(box, &cursor->loc))) {
> nsfb_cursor_clear(nsfb, cursor);
> }
> */
>
> return 0;
> }

The framebuffer front end normally renders its own mouse pointer.
From this, I guess you're relying on the pointer from the KolibriOS
desktop.

This should work for the most part, but I'm not sure you'll be able to
do things like make the pointer change shape when hovering over a link
unless we extend the nsfb surface API.

--

Michael Drake http://www.codethink.co.uk/