[haiku-commits] Re: r38354 - in haiku/trunk: headers/private/graphics/intel_extreme src/add-ons/accelerants/intel_extreme
- From: "Adrien Destugues" <adestugu@xxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Wed, 25 Aug 2010 17:43:23 +0200
> Hi Adrien,
>
> just some coding-style remarks, and thanks for improving the Intel
> graphics driver!
I was just tired of not having native resolution on my laptop. Now it
works (as well as many others resolutions as low as 640x350).
> > if (intel_propose_display_mode(&target, mode, mode))
> > return B_BAD_VALUE;
> > + // TODO : it may be acceptable to continue when using
> > panel fitting,
> > + // since the data from propose_display_mode will not
> > actually get any
> > + // use
>
> The if block should get parenthesis now, since the comment makes it a
> multi-line if-block.
The TODO refers to the whole if block above, so I see it as outside of
the block.
> > if ((gInfo->head_mode& HEAD_MODE_B_DIGITAL) != 0) {
> > + // For LVDS panels, we actuallyalways set the native
> > mode in hardware
> > + // Then we use the panel fitter to scale the picture to
> > that.
> > + display_mode hardware_target;
> > + bool needs_scaling = false;
>
> The correct variable names would be "hardwareTarget" and
> "needsScaling".
Yes. I get confused by all the underscore in struct members and
register names in this file.
> > + // TODO : use the info from EDID 1.2
> > + /* Each detailed_monitor has a type, find the first
> > one which is
> > + * actually a detailed_timing
> > + hardware_target.virtual_width = gInfo->edid_info
> > + .detailed_monitor[0].data.detailed_timing.
> > h_visible;
> > + hardware_target.virtual_height = gInfo->edid_info
> > + .detailed_monitor[0].data.detailed_timing.
> > v_visible;
> > + */
>
> Why the mix of different comment styles?
This will be removed in my next commit anyway.
> > - write32(INTEL_PANEL_FIT_CONTROL, 0);
> > + // Disable panel fitting, but enable 8 to 6-bit
> > dithering
>
> The sentence casing in comments is preferred (I think), but you do it
> inconsistently, you start many other comments that read like
> sentences
> lowercase (nitpicking, I know).
The existing comments are already inconsistent. I'll fix if I notice.
--
Adrien.
Other related posts:
