Hi Adam,
This is my feedback on the work you did so far in your
branches/adamblokus/netsurf branch. I'm quite pleased with your
contribution. :-) Apart from the necessary pdf/font_haru.h change (see
below for details) I don't see the following feedback as a showstopper
to have a merge to trunk which I will do shortly.
I will spend some more time playing with the PDF generation itself the
coming period and might then come up with more detailed feedback on that
part.
A general comment : in ANSI C you better declare functions without parameters
using an explicit 'void' instead of nothing specified (which is however
perfectly possible in C++). Doing a build on your branch gives me
several instances of:
./desktop/printer.h:36: warning: function declaration isn't a prototype
So:
> struct printer{
> struct plotter_table *plotter;
>
> bool (*print_begin) (struct print_settings*);
>
> bool (*print_next_page)();
bool (*print_next_page)(void);
^^^^
>
> void (*print_end)();
void (*print_end)(void);
^^^^
> };
Please go through your code as this is the case elsewhere too.
Now some feedback in more detail:
> Index: render/loosen.c
> [...]
> +bool loosen_position_static(struct box *box, int width, int cx,
> + struct content *content)
> +{
> + assert(box->style);
> +
> + if (box->style->position == CSS_POSITION_ABSOLUTE /*&&
> + box->x + box->width > width*/) {
Why is the 2nd part of the test commented out ? If you want to leave it like
that for good reason, it deserves a comment explaining it.
> + box->style->position = CSS_POSITION_NOT_SET;
> + }
> +
> + return true;
> +}
> +
> [...]
> +/*pass 1 -
> + */
I guess this is a left over, no ?
> [...]
> Index: render/html.h
> ===================================================================
> --- render/html.h (.../trunk/netsurf) (revision 4712)
> +++ render/html.h (.../branches/adamblokus/netsurf) (revision 4712)
> @@ -130,6 +130,7 @@
>
> struct box *layout; /**< Box tree, or 0. */
> colour background_colour; /**< Document background colour. */
> + struct font_functions * font_func;
I guess this can be 'const struct font_functions *' expressing the fact
we're never going to write to that struct via that pointer. And drop
the space after the '*'.
> [...]
> Index: render/layout.c
> ===================================================================
> --- render/layout.c (.../trunk/netsurf) (revision 4712)
> +++ render/layout.c (.../branches/adamblokus/netsurf) (revision 4712)
> [...]
> @@ -116,10 +121,11 @@
> {
> bool ret;
> struct box *doc = content->data.html.layout;
> + struct font_functions *font_func = content->data.html.font_func;
When you're following above suggestion, this needs of course to follow as
"const struct font_functions *font_func = ..." as well. Likewise in several
other places.
> [...]
> Index: render/box.c
> ===================================================================
> --- render/box.c (.../trunk/netsurf) (revision 4712)
> +++ render/box.c (.../branches/adamblokus/netsurf) (revision 4712)
> [...]
> +static int box_compare_dict_elements(struct box_dict_element *a,
> + struct box_dict_element *b);
> +
> +int box_compare_dict_elements(struct box_dict_element *a,
> + struct box_dict_element *b)
I think this would be better:
int box_compare_dict_elements(const struct box_dict_element *a,
const struct box_dict_element *b)
> +{
> + return (int)((*a).old) - (int)((*b).old);
> +}
Brr, casting a pointer to integer is wrong. Normally you can use type
intptr_t for this but as you need an int to interface with qsort(),
you better go for code like:
return (a->old < b->old) ? -1 : ((a->old > b->old) ? 1 : 0);
> [...]
> + qsort(box_dict, (box_dict_end - box_dict), sizeof(struct
> box_dict_element),
> + (int (*)(const void *, const void *))box_compare_dict_elements);
I don't think that casting won't be necessary anymore if you follow above
const suggestion. Likewise for a couple of other places.
> [...]
> + (**dict).old = old_box;
> + (**dict).new = new_box;
(*dict)->old = ... looks a bit easier to read IMHO.
> + (*dict)++;
> +}
> [...]
> Index: gtk/gtk_print.c
> [...]
> +static bool nsgtk_print_plot_clg(colour c);
> +static bool nsgtk_print_plot_rectangle(int x0, int y0, int width, int height,
> + int line_width, colour c, bool dotted, bool
> dashed);
> +static bool nsgtk_print_plot_line(int x0, int y0, int x1, int y1, int width,
> + colour c, bool dotted, bool dashed);
> +static bool nsgtk_print_plot_polygon(int *p, unsigned int n, colour fill);
> +static bool nsgtk_print_plot_path(float *p, unsigned int n, colour fill,
> float width,
> + colour c, float *transform);
> +static bool nsgtk_print_plot_fill(int x0, int y0, int x1, int y1, colour c);
> +static bool nsgtk_print_plot_clip(int clip_x0, int clip_y0,
> + int clip_x1, int clip_y1);
> +static bool nsgtk_print_plot_text(int x, int y, const struct css_style
> *style,
> + const char *text, size_t length, colour bg, colour
> c);
> +static bool nsgtk_print_plot_disc(int x, int y, int radius, colour c, bool
> filled);
> +static bool nsgtk_print_plot_arc(int x, int y, int radius, int angle1, int
> angle2,
> + colour c);
> +static bool nsgtk_print_plot_bitmap(int x, int y, int width, int height,
> + struct bitmap *bitmap, colour bg, struct content
> *content);
> +static bool nsgtk_print_plot_bitmap_tile(int x, int y, int width, int height,
> + struct bitmap *bitmap, colour bg,
> + bool repeat_x, bool repeat_y, struct content *content);
Two TABs as start continuation of a function declaration/definition.
> [...]
> +static bool gtk_print_font_paint(const struct css_style *style,
> + const char *string, size_t length,
> + int x, int y, colour c);
Likewise.
> [...]
> +struct printer gtk_printer= {
> + &nsgtk_print_plotters,
> + gtk_print_begin,
> + gtk_print_next_page,
> + gtk_print_end
> +};
I don't know this code well, but can't this be made "const struct printer" ?
Or even "static const struct printer" ?
> [...]
> +bool nsgtk_print_plot_line(int x0, int y0, int x1, int y1, int width,
> + colour c, bool dotted, bool dashed)
> +{
> + nsgtk_print_set_colour(c);
> + if (dotted)
> + nsgtk_print_set_dotted();
> + else if (dashed)
> + nsgtk_print_set_dashed();
> + else
> + nsgtk_print_set_solid();
> +
> +
> + if (width == 0)
> + width = 1;
> +
> + cairo_set_line_width(gtk_print_current_cr, width);
> + cairo_move_to(gtk_print_current_cr, x0, y0 - 0.5);
> + cairo_line_to(gtk_print_current_cr, x1, y1 - 0.5);
Could someone explain the 0.5 subtraction ? Just curious.
> + cairo_stroke(gtk_print_current_cr);
> +
> + return true;
> +}
> [...]
> Index: gtk/gtk_print.h
> ===================================================================
> --- gtk/gtk_print.h (.../trunk/netsurf) (revision 0)
> +++ gtk/gtk_print.h (.../branches/adamblokus/netsurf) (revision 4712)
> [...]
> +extern struct printer gtk_printer;
> +
> +struct content *content_to_print;
I think 'extern' is missing. You're creating multiple instances of this
variable for each C source including this header. And make sure
content_to_print is defined in a suitable C source if that's not the case.
> [...]
> Index: gtk/res/license
> ===================================================================
> --- gtk/res/license (.../trunk/netsurf) (revision 4712)
> +++ gtk/res/license (.../branches/adamblokus/netsurf) (revision 4712)
> @@ -1,332 +0,0 @@
> -The source code, documentation, translatable messages files and UI
> -definitions contained within NetSurf are licensed under the following terms:
> [...]
This file is no longer available on your branch. svnmerge troubles ?
> [...]
> Index: Docs/BUILDING-BeOS
> ===================================================================
> --- Docs/BUILDING-BeOS (.../trunk/netsurf) (revision 4712)
> +++ Docs/BUILDING-BeOS (.../branches/adamblokus/netsurf)
> (revision 4712)
> @@ -1,139 +0,0 @@
> ---------------------------------------------------------------------------------
> - Build Instructions for BeOS and Haiku NetSurf 06 June
> 2008
> ---------------------------------------------------------------------------------
> -
> [...]
Likewise.
> [...]
> Index: Docs/ideas/render-library.txt
> ===================================================================
> --- Docs/ideas/render-library.txt (.../trunk/netsurf) (revision 4712)
> +++ Docs/ideas/render-library.txt (.../branches/adamblokus/netsurf)
> (revision 4712)
> @@ -1,121 +0,0 @@
> -Rendering library
> -=================
> [...]
Likewise.
> [...]
> Index: pdf/pdf_printer.h
> ===================================================================
> --- pdf/pdf_printer.h (.../trunk/netsurf) (revision 0)
> +++ pdf/pdf_printer.h (.../branches/adamblokus/netsurf) (revision 4712)
> [...]
> +#ifndef NETSURF_PDF_PRINTER_H
> +#define NETSURF_PDF_PRINTER_H
> +
> +#include "desktop/printer.h"
> +#include "pdf/pdf_plotters.h"
> +
> +struct printer pdf_printer= {
> + &pdf_plotters,
> + pdf_begin,
> + pdf_next_page,
> + pdf_end
> +};
This is not good. Every C source including this header will create an
instance of 'pdf_printer' variable. The definition needs to be in a
C source and the declaration (assuming the variable is needed in another
C source) in this header (having 'extern').
> [...]
> Index: pdf/font_haru.c
> ===================================================================
> --- pdf/font_haru.c (.../trunk/netsurf) (revision 0)
> +++ pdf/font_haru.c (.../branches/adamblokus/netsurf) (revision 4712)
> [...]
> +#define FONT_HARU_DEBUG true
This does do two things, first enable the error handler and secondly
enable disable the LOG() during PDF export. The first usage is fine
to always have in installed, even in non-debug builds.
However the current error handler implementation requires some tuning.
Basically I would remove FONT_HARU_DEBUG test which conditionally
installs the error handler and always have it installed.
For the second usage we want to have this 'false' in our non-debug
builds. So basically I would true -> false.
> [...]
> +#include "css/css.h"
> +#include "hpdf.h"
> +#include <string.h>
> +#include "render/font.h"
> +#include "pdf/font_haru.h"
> +#include "utils/log.h"
It's nice to have the non-NetSurf headers included first and using
<> syntax for them too:
#include <string.h>
#include <hpdf.h>
#include "css/css.h"
#include "render/font.h"
#include "pdf/font_haru.h"
#include "utils/log.h"
> +
> +
> +/**
> + * Haru error handler - for normal use it is turned off,
> + * for debugging purposes - it immediately exits the program on the first
> error,
> + * as it would otherwise flood the user with all resulting complications,
> + * covering the most important error source.
> + */
> +static void error_handler(HPDF_STATUS error_no,
> + HPDF_STATUS detail_no,
> + void *user_data)
> +{
> + printf ("ERROR: in font_haru \n\terror_no=%x\n\tdetail_no=%d\n",
> + (HPDF_UINT)error_no,
> + (HPDF_UINT)detail_no);
> +
> + exit(1);
> +}
Use LOG() for reporting the error message. Is there anyway to recover
instead of exit() (which is a no-go as this makes your browser go
away) ? I believe Haru suggests setjmp/longjmp.
> +
> +static bool haru_nsfont_init(HPDF_Doc *pdf, HPDF_Page *page,
> + const char *string, char **string_nt, int length)
> +{
> +
> +#ifdef FONT_HARU_DEBUG
> + *pdf = HPDF_New(error_handler, NULL);
> +#else
> + *pdf = HPDF_New(NULL, NULL);
> +#endif
Always have an error_handler installed as this can give a much better
user experience in case of errors/problems during PDF generation.
> + *page = HPDF_AddPage(*pdf);
> + if (*page == NULL)
> + return false;
> +
> + *string_nt = malloc((length + 1) * sizeof(char));
> + if (*string_nt == NULL)
> + return false;
> +
> + strncpy(*string_nt, string, length);
memcpy() ? As I don't think strncpy() does something more interesting
than memcpy() for this particular case.
> + (*string_nt)[length] = '\0';
> + return true;
> +}
> +
> +/**
> + * Measure the width of a string.
> + *
> + * \param style css_style for this text, with style->font_size.size ==
> + * CSS_FONT_SIZE_LENGTH
> + * \param string string to measure (no UTF-8 currently)
> + * \param length length of string
> + * \param width updated to width of string[0..length]
> + * \return true on success, false on error and error reported
> + */
> +bool haru_nsfont_width(const struct css_style *style,
> + const char *string, size_t length,
> + int *width)
> +{
> + HPDF_Doc pdf;
> + HPDF_Page page;
> + char *string_nt;
> + HPDF_REAL width_real;
> +
> + if (length == 0) {
> + *width = 0;
> + return true;
> + }
> +
> + if (!haru_nsfont_init(&pdf, &page, string, &string_nt, length))
> + return false;
pdf & string_nt get allocated above....
> +
> + if (!haru_nsfont_apply_style(style, pdf, page, NULL))
> + return false;
...but if this if() returns you forget to free those.
> +
> + width_real = HPDF_Page_TextWidth(page, string_nt);
> + *width = width_real;
> +
> +#ifdef FONT_HARU_DEBUG
> + LOG(("Measuring string: %s ; Calculated width: %f %i",string_nt,
> width_real, *width));
> +#endif
> + free(string_nt);
> + HPDF_Free(pdf);
> +
> +
> + return true;
> +}
> +
> +
> +/**
> + * Find the position in a string where an x coordinate falls.
> + *
> + * \param style css_style for this text, with style->font_size.size ==
> + * CSS_FONT_SIZE_LENGTH
> + * \param string string to measure (no UTF-8 currently)
> + * \param length length of string
> + * \param x x coordinate to search for
> + * \param char_offset updated to offset in string of actual_x,
> [0..length]
> + * \param actual_x updated to x coordinate of character closest to x
> + * \return true on success, false on error and error reported
> + */
> +
> +bool haru_nsfont_position_in_string(const struct css_style *style,
> + const char *string, size_t length,
> + int x, size_t *char_offset, int *actual_x)
> +{
> + HPDF_Doc pdf;
> + HPDF_Page page;
> + char *string_nt;
> + HPDF_UINT offset;
> + HPDF_REAL real_width;
> + HPDF_REAL width;
> +
> +
> + if (!haru_nsfont_init(&pdf, &page, string, &string_nt, length))
> + return false;
Likewise comment on freeing pdf & string_nt when returning early.
I see more instances of this further in the code which I won't
repeat.
> +
> + if (!HPDF_Page_SetWidth(page, x))
> + return false;
> +
> + if (!haru_nsfont_apply_style(style, pdf, page, NULL))
> + return false;
> +
> + offset = HPDF_Page_MeasureText(page, string_nt, x,
> + HPDF_FALSE, &real_width);
> +
> +
> + if (real_width < x)
> + *char_offset = offset;
> + else/*real_width == x*/
> + *char_offset = offset - 1;
This begs for assert()s (looking at the code as an outsider), i.e.
if (real_width < x)
*char_offset = offset;
else {
assert(real_width == x);
assert(offset > 0);
*char_offset = offset - 1;
}
assert(fabs(real_width - x) < FLT_EPSILON) is probably better
(you might need #include <float.h>).
> +
> + /*TODO: this is only the right edge of the character*/
> + *actual_x = real_width;
> +
> +#ifdef FONT_HARU_DEBUG
> + LOG(("Position in string: %s at x: %i; Calculated position: %i",
> + string_nt, x, *char_offset));
> +#endif
> + free(string_nt);
> + HPDF_Free(pdf);
> +
> + return true;
> +}
> [...]
> +bool haru_nsfont_apply_style(const struct css_style *style,
> + HPDF_Doc doc, HPDF_Page page,
> + HPDF_Font *font)
> [...]
> + if (font != NULL) {
> + pdf_font = HPDF_GetFont(doc, font_name, "StandardEncoding");
> + if (pdf_font == NULL)
> + return false;
> + *font = pdf_font;
> + }
> + else {
> +
> + if (style->font_size.value.length.unit == CSS_UNIT_PX)
> + size = style->font_size.value.length.value;
> + else
> + size = css_len2pt(&style->font_size.value.length,
> style);
> +
> + size = size * 1.45;
What's this 1.45 value ?
> +
> + pdf_font = HPDF_GetFont(doc, font_name, "StandardEncoding");
> + if (pdf_font == NULL)
> + return false;
> + HPDF_Page_SetFontAndSize(page, pdf_font, size);
> + }
> +
> + return true;
> +}
> Index: pdf/pdf_plotters.c
> ===================================================================
> --- pdf/pdf_plotters.c (.../trunk/netsurf) (revision 0)
> +++ pdf/pdf_plotters.c (.../branches/adamblokus/netsurf)
> (revision 4712)
> [...]
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "desktop/plotters.h"
> +#include "desktop/print.h"
> +#include "pdf/pdf_plotters.h"
> +#include "utils/log.h"
> +#include "utils/utils.h"
> +#include "image/bitmap.h"
> +
> +#include "hpdf.h"
Move this as "#include <hpdf.h>" after the string.h include.
> +#include "font_haru.h"
> +
> [...]
> +/*PDF Plotter - current doc,page and font*/
> +static HPDF_Doc pdf_doc;
> +static HPDF_Page pdf_page;
> +static HPDF_Font pdf_font;
> +
> +/*PDF Page size*/
> +static HPDF_REAL page_height, page_width;
> +
> +/*Remeber if pdf_plot_clip was invoked for current page*/
> +static bool page_clipped;
> +
> +static struct print_settings* settings;
> +static float text_margin;
Short comment what this value does please.
> +
> +extern struct plotter_table plot;
If needed, this needs to be found in a header instead of having it
declared here. I suspect you can remove this.
> +
> +const struct plotter_table pdf_plotters = {
> + pdf_plot_clg,
> + pdf_plot_rectangle,
> + pdf_plot_line,
> + pdf_plot_polygon,
> + pdf_plot_fill,
> + pdf_plot_clip,
> + pdf_plot_text,
> + pdf_plot_disc,
> + pdf_plot_arc,
> + pdf_plot_bitmap,
> + pdf_plot_bitmap_tile,
> + NULL,
> + NULL,
> + pdf_plot_flush,
> + pdf_plot_path
> +};
static possible ?
> [...]
> +bool pdf_plot_fill(int x0, int y0, int x1, int y1, colour c){
> +
> +#ifdef PDF_DEBUG
> + LOG(("%d %d %d %d %f %X", x0, y0, x1, y1, page_height-y0, c));
> +#endif
> +
> + /*Normalize boundaries of the area - to prevent overflows*/
> + x0 = min(max(x0, 0), page_width);
> + y0 = min(max(y0, 0), page_height);
> + x1 = min(max(x1, 0), page_width);
> + y1 = min(max(y1, 0), page_height);
I know this code is also there in other plotters but what kind of 'overflow'
are we avoiding here ? And I don't see similart code elsewhere, like
pdf_plot_line(), pdf_plot_polygon() etc ? Shouldn't we do the same ?
> [...]
> +bool pdf_plot_text(int x, int y, const struct css_style *style,
> + const char *text, size_t length, colour bg, colour c){
> +#ifdef PDF_DEBUG
> + LOG((". %d %d %s", x, y, text));
> +#endif
> [...]
> + word = (char*) malloc( sizeof(char) * (length+1) );
> + if (word == NULL)
> + return false;
> +
> + strncpy(word, text, length);
memcpy()
> + word[length] = '\0';
> [...]
> +bool pdf_plot_bitmap(int x, int y, int width, int height,
> + struct bitmap *bitmap, colour bg, struct content *content){
> +
> + HPDF_Image image;
> +
> +#ifdef PDF_DEBUG
> + LOG(("%d %d %d %d %X %X %X", x, y, width, height,
> + bitmap, bg, content));
> +#endif
> +
> + image = pdf_extract_image(bitmap, content);
> +
> + if (image) {
> + HPDF_Page_DrawImage(pdf_page, image,
> + x, page_height-y-height,
> + width, height);
> + return true;
> + }
> + else
> + return false;
This can be made slightly better readable as:
image = pdf_extract_image(bitmap, content);
if (!image)
return false;
HPDF_Page_DrawImage(pdf_page, image,
x, page_height-y-height,
width, height);
return true;
> +}
> +
> +bool pdf_plot_bitmap_tile(int x, int y, int width, int height,
> + struct bitmap *bitmap, colour bg,
> + bool repeat_x, bool repeat_y, struct content *content){
> +
> + HPDF_Image image;
> +
> +#ifdef PDF_DEBUG
> + LOG(("%d %d %d %d %X %X %X", x, y, width, height,
> + bitmap, bg, content));
> +#endif
> +
> + image = pdf_extract_image(bitmap, content);
> +
> + if (image) {
> + /*The position of the next tile*/
> + HPDF_REAL current_x, current_y ;
> + HPDF_REAL max_width, max_height;
> +
> + max_width = (repeat_x ? page_width : width);
> + max_height = (repeat_y ? page_height: height);
> +
> + current_y=0;
> + while (current_y < max_height) {
> + current_x=0;
> + while (current_x < max_width) {
> + HPDF_Page_DrawImage(pdf_page, image,
> + current_x,
> +
> page_height-current_y-height,
> + width, height);
> + current_x += width;
> + }
> + current_y += height;
> + }
2x for() i.s.o. the 2 while()s is better readable IMHO.
> +
> + return true;
> + }
> + else
> + return false;
> +
> + return true;
> +}
> +
> +HPDF_Image pdf_extract_image(struct bitmap *bitmap, struct content *content){
> + HPDF_Image image = NULL,smask;
> + char *img_buffer, *rgb_buffer, *alpha_buffer;
> + int img_width, img_height, img_rowstride;
> + int i, j;
> +
> + if (content) {
> + /*Not sure if I don't have to check if downloading has been
> + finished.
> + Other way - lock pdf plotting while fetching a website
> + */
This requires feedback from the other NetSurf developers. It is a valid
concern which needs to be addressed or made clear that this isn't an issue.
> + switch(content->type){
> + /*Handle "embeddable" types of images*/
> + case CONTENT_JPEG:
> + image = HPDF_LoadJpegImageFromMem(pdf_doc,
> + content->source_data,
> + content->total_size);
> + break;
> +
> + /*Disabled until HARU PNG support will be more stable.
What's the issue here ?
> +
> + case CONTENT_PNG:
> + image = HPDF_LoadPngImageFromMem(pdf_doc,
> + content->source_data,
> + content->total_size);
> + break;*/
> + }
> + }
> +
> [...]
> +bool pdf_begin(struct print_settings* print_settings)
> +{
> +
> + pdf_doc = NULL;
> +
> +#ifdef PDF_DEBUG
> + pdf_doc = HPDF_New(error_handler, NULL);
> +#else
> + pdf_doc = HPDF_New(NULL, NULL);
> +#endif
I would always have an error_handler.
> [...]
> +void pdf_end()
> +{
> +#ifdef PDF_DEBUG
> + LOG(("pdf_end begins"));
> + if (pdf_page != NULL) {
> + HPDF_Page_GRestore(pdf_page);
> + if (page_clipped)
> + HPDF_Page_GRestore(pdf_page);
> + pdf_plot_grid(10, 10, 0xCCCCCC);
> + pdf_plot_grid(100, 100, 0xCCCCFF);
> + }
> +#endif
> +
> + if (settings->output)
> + HPDF_SaveToFile(pdf_doc, settings->output);
> + else
> + HPDF_SaveToFile(pdf_doc, "out.pdf");
Is it actual possible to have settings->output being NULL ? If so, better
give an error message upfront and don't do any PDF generation at all as
writing a file to whatever the current directory is, is not good.
> +
> + HPDF_Free(pdf_doc);
> +
> +#ifdef PDF_DEBUG
> + LOG(("pdf_end finishes"));
> +#endif
> +}
> +
> +
> +/**
> + * Haru error handler - for normal use it is turned off,
> + * for debugging purposes - it immediately exits the program on the first
> error,
> + * as it would otherwise flood the user with all resulting complications,
> + * covering the most important error source.
> +*/
> +static void error_handler(HPDF_STATUS error_no,
> + HPDF_STATUS detail_no,
> + void *user_data)
> +{
> + printf ("ERROR:\n\terror_no=%x\n\tdetail_no=%d\n",
> + (HPDF_UINT)error_no,
> + (HPDF_UINT)detail_no);
> +
> + exit(1);
> +}
Same comment as on the other error_handler().
> +
> +/**
> + * This function plots a grid - used for debug purposes to check if all
> + * elements' final coordinates are correct.
> +*/
> +void pdf_plot_grid(int x_dist, int y_dist, unsigned int colour)
> +{
> + int i;
> +
> + for (int i = x_dist ; i < page_width ; i += x_dist)
> + pdf_plot_line(i, 0, i, page_height, 1, colour, false, false);
> +
> + for (int i = y_dist ; i < page_height ; i += x_dist)
> + pdf_plot_line(0, i, page_width, i, 1, colour, false, false);
> +
> +}
If it is only needed for debugging, wrap this code in the same #define
condition as to enable/disable the call to this (PDF_DEBUG I think).
> [...]
> Index: pdf/font_haru.h
> ===================================================================
> --- pdf/font_haru.h (.../trunk/netsurf) (revision 0)
> +++ pdf/font_haru.h (.../branches/adamblokus/netsurf) (revision 4712)
> [...]
> +bool haru_nsfont_apply_style(const struct css_style *style,
> + HPDF_Doc doc, HPDF_Page page,
> + HPDF_Font *font);
> +
> +const struct font_functions haru_nsfont;
Not good as this defines an instance haru_nsfont for each C file including
this header. I.e. an 'extern' is missing. Actually I had to fix this for
the RISC OS build as otherwise PDF export doesn't work at all (it was
reading garbage function pointers).
> +
> +
> +#endif
> [...]
> +/**
> + * Generates one of the predefined print settings sets.
> + * \return print_settings in case if successful, NULL if unknown
> configuration \
> + * or lack of memory.
> + */
> +struct print_settings *print_make_settings(print_configuration configuration)
> +{
> +
> + struct print_settings *settings;
> +
> + switch (configuration){
> + case DEFAULT:
> + settings = (struct print_settings*)
> + malloc(sizeof (struct print_settings) );
> +
> + if (settings == NULL)
> + return NULL;
> +
> + settings->page_width = 595;
> + settings->page_height = 840;
> + settings->copies = 1;
> + settings->scale = 0.7;
Is there a specific reason for .7 scale ?
> +
> + settings->margins[MARGINLEFT] = 30;
> + settings->margins[MARGINRIGHT] = 30;
> + settings->margins[MARGINTOP] = 30;
> + settings->margins[MARGINBOTTOM] = 30;
> +
> + settings->margins[MARGINTEXT] = 10;
> +
> + settings->output = NULL;
> + settings->font_func = &haru_nsfont;
> +
> + break;
> +
> + default:
> + return NULL;
> + }
> +
> + return settings;
> +}
As last remark : do a full build of NetSurf had have a look at the
warnings generated of the code you've touched and try to deal with it.
On the longer run we want to go for warning-free builds.
Regards,
John.
--
John Tytgat
[EMAIL PROTECTED]
